-
Notifications
You must be signed in to change notification settings - Fork 171
tests/kola/commonlib: Fix get_fcos_stream for container case #3964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: testing-devel
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the get_fcos_stream function to correctly parse the stream from the container image's annotations. The implementation has been changed to handle a nested JSON structure within the rpm-ostree status output. My review includes a suggestion to optimize the jq command by combining the two separate calls into a single, more efficient one.
59020b5 to
9404ff7
Compare
Get the stream from the `com.coreos.stream` annotation from the container image.
9404ff7 to
070d3cf
Compare
|
Thanks, updated. |
| get_fcos_stream() { | ||
| rpm-ostree status -b --json | jq -r '.deployments[0]["base-commit-meta"]["fedora-coreos.stream"]' | ||
| rpm-ostree status -b --json \ | ||
| | jq -r '.deployments[0]."base-commit-meta"."ostree.manifest" | fromjson | .annotations."com.coreos.stream"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what JB was saying is that we are moving towards the label being the source of truth (not the annotation) so:
[core@cosa-devsh ~]$ rpm-ostree status -b --json | jq --raw-output '.deployments[0]."base-commit-meta"."ostree.container.image-config" | fromjson | .config.Labels."com.coreos.stream"'
testing-devel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum indeed. I don't know why it worked for me before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still have the annotation for now, so that's why it works either way
Get the stream from the
com.coreos.streamannotation from the container image.