Skip to content
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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Nov 8, 2024
Merged

stf-run-ci: get operator info from CSV instead of image metadata #643

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d706ed2
stf-run-ci: get operator info from CSV instead of image metadata
compi-migui Nov 7, 2024
4f8ff58
[stf-run-ci][create_catalog] Fetch files to ansible controller before…
elfiesmelfie Nov 7, 2024
81668dd
[stf-run-ci][create_catalog] Default package name in local build only
compi-migui Nov 8, 2024
ac1cbfd
[stf-run-ci][create_catalog] Fixup Zuul workdir reference
compi-migui Nov 8, 2024
ed12f4a
[stf-run-ci][create_catalog] Fixup SGO bundle local build package name
compi-migui Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions build/stf-run-ci/tasks/create_catalog.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,60 @@
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
Copy link
Collaborator

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

Copy link
Contributor Author

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

when: sto_bundle_info is defined and sto_bundle_info.package_name is not defined
ansible.builtin.set_fact:
sto_bundle_info: "{{ sto_bundle_info | combine({'package_name':'service-telemetry-operator.v%s'|format(sto_bundle_info.operator_bundle_version)}) }}"
sgo_bundle_info: "{{ sgo_bundle_info | combine({'package_name':'smart-gateway-operator.v%s'|format(sto_bundle_info.operator_bundle_version)}) }}"

- name: Create info variables from provided pre-built bundles (deploy from bundles)
when: __deploy_from_bundles_enabled | bool and not __local_build_enabled | bool
block:
- name: Get STO operator bundle info
ansible.builtin.command: oc image info {{ __service_telemetry_bundle_image_path }}
register: sto_prebuilt_image_info

- name: Get SGO operator bundle info
ansible.builtin.command: oc image info {{ __smart_gateway_bundle_image_path }}
register: sgo_prebuilt_image_info
- name: Get STO operator bundle CSV file
ansible.builtin.command: oc image extract {{ __service_telemetry_bundle_image_path }} --file /manifests/*clusterserviceversion.yaml
args:
chdir: "{{ base_dir }}/working/service-telemetry-framework-index/"

- name: Get SGO operator bundle CSV file
ansible.builtin.command: oc image extract {{ __smart_gateway_bundle_image_path }} --file /manifests/*clusterserviceversion.yaml
args:
chdir: "{{ base_dir }}/working/service-telemetry-framework-index/"

Copy link
Collaborator

@elfiesmelfie elfiesmelfie Nov 7, 2024 •
edited
Loading

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.

https://logserver.rdoproject.org/43/643/3d353512a7e5bf574ca453b42a771b99df896dd7/github-check/stf-crc-ocp_416-nightly_bundles-index_deploy/3ac2688/zuul-info/inventory.yaml

For the fetch module, the src is the target node for this play, the dest is on the ansible controller

Suggested change
- 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

Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

@elfiesmelfie elfiesmelfie Nov 8, 2024 •
edited
Loading

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

- name: Read STO bundle CSV file contents
ansible.builtin.include_vars:
file: "{{ base_dir }}/working/service-telemetry-framework-index/service-telemetry-operator.clusterserviceversion.yaml"
compi-migui marked this conversation as resolved.
Show resolved Hide resolved
name: sto_prebuilt_bundle_csv

- name: Read SGO bundle CSV file contents
ansible.builtin.include_vars:
file: "{{ base_dir }}/working/service-telemetry-framework-index/smart-gateway-operator.clusterserviceversion.yaml"
compi-migui marked this conversation as resolved.
Show resolved Hide resolved
name: sgo_prebuilt_bundle_csv

- name: Get STO and SGO bundle package names (from CSV)
ansible.builtin.set_fact:
sto_prebuilt_bundle_package_name: "{{ sto_prebuilt_bundle_csv.metadata.name }}"
sgo_prebuilt_bundle_package_name: "{{ sgo_prebuilt_bundle_csv.metadata.name }}"

- name: Get STO and SGO bundle versions (from metadata)
- name: Get STO and SGO bundle versions (from CSV)
ansible.builtin.set_fact:
sto_prebuilt_bundle_version: "{{ sto_prebuilt_image_info.stdout_lines[-1] | split('=') | last }}"
sgo_prebuilt_bundle_version: "{{ sgo_prebuilt_image_info.stdout_lines[-1] | split('=') | last }}"
sto_prebuilt_bundle_version: "{{ sto_prebuilt_bundle_csv.spec.version }}"
sgo_prebuilt_bundle_version: "{{ sgo_prebuilt_bundle_csv.spec.version }}"

- name: Get STO and SGO bundle tags (from name)
- name: Get STO and SGO bundle image tags (from name)
ansible.builtin.set_fact:
sto_prebuilt_bundle_tag: "{{ __service_telemetry_bundle_image_path | split(':') | last }}"
sgo_prebuilt_bundle_tag: "{{ __smart_gateway_bundle_image_path | split(':') | last }}"

- name: Set info variables from provided pre-built bundles
ansible.builtin.set_fact:
sto_bundle_info:
'package_name': "{{ sto_prebuilt_bundle_package_name }}"
'bundle_default_channel': "{{ stf_channel }}"
'bundle_channels': "{{ stf_channel }}"
'operator_bundle_version': "{{ sto_prebuilt_bundle_version }}"
'operator_bundle_tag': "{{ sto_prebuilt_bundle_tag }}"
sgo_bundle_info:
'package_name': "{{ sgo_prebuilt_bundle_package_name }}"
'bundle_default_channel': "{{ stf_channel }}"
'bundle_channels': "{{ stf_channel }}"
'operator_bundle_version': "{{ sgo_prebuilt_bundle_version }}"
Expand Down
4 changes: 2 additions & 2 deletions build/stf-run-ci/templates/index-yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ schema: olm.channel
package: service-telemetry-operator
name: {{ sto_bundle_info.bundle_channels }}
entries:
- name: service-telemetry-operator.v{{ sto_bundle_info.operator_bundle_version }}
- name: {{ sto_bundle_info.package_name }}
---
defaultChannel: {{ sgo_bundle_info.bundle_default_channel }}
name: smart-gateway-operator
Expand All @@ -17,4 +17,4 @@ schema: olm.channel
package: smart-gateway-operator
name: {{ sgo_bundle_info.bundle_channels }}
entries:
- name: smart-gateway-operator.v{{ sgo_bundle_info.operator_bundle_version }}
- name: {{ sgo_bundle_info.package_name }}