-
Notifications
You must be signed in to change notification settings - Fork 13
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
stf-run-ci: get operator info from CSV instead of image metadata #643
Conversation
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3da555601dd94072ab1fdcac4ac39fa1 ❌ stf-crc-ocp_414-local_build RETRY_LIMIT in 23m 22s |
cb39036
to
3d35351
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/30230ee2acf2449ea7fc72079b45380b ❌ stf-crc-ocp_414-local_build RETRY_LIMIT in 28m 22s |
recheck |
3d35351
to
a3cfcab
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e854741365df4eb1b18d96cd99341240 ❌ stf-crc-ocp_414-local_build RETRY_LIMIT in 27m 01s |
OLM cares about CSV contents, not image metadata. Getting operator package information directly from the CSV guarantees that the index image we generate will actually work with the operator-bundles we're trying to test.
a3cfcab
to
d706ed2
Compare
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/c6639d353cd84230a85c674ee439810b ❌ stf-crc-ocp_414-local_build RETRY_LIMIT in 27m 12s |
ansible.builtin.command: oc image extract {{ __smart_gateway_bundle_image_path }} --file /manifests/*clusterserviceversion.yaml | ||
args: | ||
chdir: "{{ base_dir }}/working/service-telemetry-framework-index/" | ||
|
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.
Add in a fetch between the oc image extract
and the include_vars.
You will have a different dir on the executor, which is accessible through the zuul.executor.work_root
var.
For the fetch module, the src is the target node for this play, the dest is on the ansible controller
- name: Set the csv_dest based on whether zuul is used or not | |
ansible.builtin.set_fact: | |
csv_dest: "{{ zuul.executor.work_dir if zuul is defined else base_dir + '/working/service-telemetry-framework-index/' }}" | |
- name: "Put the CSV files onto the ansible controller, so we can include_vars" | |
ansible.builtin.fetch: | |
src: "{{ base_dir }}/working/service-telemetry-framework-index/service-telemetry-operator.clusterserviceversion.yaml" | |
dest: "{{ csv_dest }}/" | |
flat: yes | |
- name: "Put the CSV files onto the ansible controller, so we can include_vars" | |
ansible.builtin.fetch: | |
src: "{{ base_dir }}/working/service-telemetry-framework-index/smart-gateway-operator.clusterserviceversion.yaml" | |
dest: "{{ csv_dest }}/" | |
flat: yes | |
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.
You also need to update the filenames in the next two tasks to use this.
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.
Thanks for explaining! Reading Ansible docs along with your hints has led me to the slurp module, which might work even better here: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/slurp_module.html
We're ultimately shoving the file contents into a variable anyway, so if this works it would save us from the zuul vs not divergence and having to keep track of where the file is.
I'm giving it a try, if it doesn't work then I'll go with the way you suggested.
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 realize now you already pushed a fix so I'll leave it as is. Thank you.
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.
You made a good point with the zuul vs not divergence. I don't particularly like having to account for zuul in this, since it starts looking complicated.
With the slurp module, you need to do something like:
- slurp:
file: ...
register: slurped_file
- set_fact:
my_var: "{{ slurped_file.content | b64decode | from_yaml }}"
I can provide some playbook for testing that, if you want to play with it for learning.
I can also consider adding the replacement later, I trust that the CI job has coverage for this, since one of our test scenarios is bundle+index deploy
… trying to load vars
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e939de954a0d4ec4bfe15c2484928b13 ✔️ stf-crc-ocp_414-local_build SUCCESS in 40m 04s |
@@ -13,35 +13,76 @@ | |||
sto_bundle_info: "{{ generate_bundle_sto.stdout_lines[-1] | from_json }}" | |||
sgo_bundle_info: "{{ generate_bundle_sgo.stdout_lines[-1] | from_json }}" | |||
|
|||
- name: Generate default package names if not present |
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.
Add this step inside the "Create info variables from provided pre-built bundles (deploy from bundles)" group of steps
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 it's the opposite problem: the "deploy from bundles" group gets the package_name from the CSV, so this shouldn't run in that case at all. Changing the when:
to __local_build_enabled | bool and not __deploy_from_bundles_enabled | bool
so it only runs for local builds
Local builds get the default package name, non-local ones will get it from the actual bundle's CSV.
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/f2ef8b3429304b859bf677d197ab1056 ❌ stf-crc-ocp_414-local_build RETRY_LIMIT in 25m 20s |
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4c61c418fdab4df09d6e4841839e3ff1 ❌ stf-crc-ocp_414-local_build RETRY_LIMIT in 25m 18s |
* stf-run-ci: get operator info from CSV instead of image metadata OLM cares about CSV contents, not image metadata. Getting operator package information directly from the CSV guarantees that the index image we generate will actually work with the operator-bundles we're trying to test. * [stf-run-ci][create_catalog] Fetch files to ansible controller before trying to load vars * [stf-run-ci][create_catalog] Default package name in local build only Local builds get the default package name, non-local ones will get it from the actual bundle's CSV. * [stf-run-ci][create_catalog] Fixup Zuul workdir reference * [stf-run-ci][create_catalog] Fixup SGO bundle local build package name --------- Co-authored-by: Emma Foley <[email protected]> (cherry picked from commit e67d43b)
… (#644) * stf-run-ci: get operator info from CSV instead of image metadata OLM cares about CSV contents, not image metadata. Getting operator package information directly from the CSV guarantees that the index image we generate will actually work with the operator-bundles we're trying to test. * [stf-run-ci][create_catalog] Fetch files to ansible controller before trying to load vars * [stf-run-ci][create_catalog] Default package name in local build only Local builds get the default package name, non-local ones will get it from the actual bundle's CSV. * [stf-run-ci][create_catalog] Fixup Zuul workdir reference * [stf-run-ci][create_catalog] Fixup SGO bundle local build package name --------- Co-authored-by: Emma Foley <[email protected]> (cherry picked from commit e67d43b)
OLM cares about CSV contents, not image metadata. Getting operator package information directly from the CSV guarantees that the index image we generate will actually work with the operator-bundles we're trying to test.