[reproducer] Add dependencies compability file for pull#3783
[reproducer] Add dependencies compability file for pull#3783evallesp wants to merge 1 commit intoopenstack-k8s-operators:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/0e59f4afe0e5455a8103a063a29d8f60 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 18m 52s |
8c7a3c0 to
9c62252
Compare
9c62252 to
4e0a7a3
Compare
8ba2e4e to
cc43255
Compare
cc43255 to
2e20d13
Compare
2e20d13 to
b036249
Compare
This file in conjuction with: * openstack-k8s-operators#3774 * openstack-k8s-operators#3772 Allows ci-framework to clone those repos (architecture was already enabled) when repositories are not already there due to zuul has not marked them as required-projects or because we're running them outside the zuul ecosystem. This is the almost the lowest in precedence, so if we still have these vars defined at zuul level or defaults-vars (passed as extra-vars) or scenarios files, this file is not taken into consideration. Precedence chain: * Zuul repos * cifmw_reproducer_repositories * cifmw_reproducer_default_repositories * Git pull logic in each of the roles Signed-off-by: Enrique Vallespi Gil <evallesp@redhat.com>
b036249 to
96502c6
Compare
|
Actually, the precedence chain in the PR description makes an assumption for repo_setup, as we need to set the var as done in tcib: https://github.com/openstack-k8s-operators/ci-framework/blob/main/scenarios/centos-9/tcib.yml#L3 As this job is pulling it from zuul: https://github.com/openstack-k8s-operators/ci-framework/blob/main/zuul.d/tcib.yaml#L10 if we set cifmw_repo_setup_src as a any "https://github" url repository, then https://github.com/openstack-k8s-operators/ci-framework/pull/3772/changes#diff-c9e567935c762e94b13ba6e23ac4b630f1f6d0ddc4ffaad77f173274980901d8L24 is overwritten. repo_setup: Takes url or local folder and move to tmp. Adding in repo-setup the versioning, we're not making the problem bigger. |
michburk
left a comment
There was a problem hiding this comment.
I see that in many cases, you are defining variables here in group_vars/all.yml, and in role defaults. For example, cifmw_kustomize_deploy_architecture_repo_version is defined both here, and in roles/kustomize_deploy/defaults/main.yml.
This is fine, and according to Ansible's var precedence, the group_vars definitions will be higher precedence than the definitions in the role defaults, so things should act as we're expecting here.
With that being said, though, I don't like moving a var with a role-specific name like cifmw_kustomize_deploy_architecture_repo_version to group_vars, and I think we can be more explicit about where the definition for a specific variable is coming from. As-is, it's confusing to have a role's default var being overwritten by some definition in a completely separate directory. We should either only define these values in group_vars, or, sticking with the cifmw_kustomize_deploy_architecture_repo_version var for example:
We could define a var in group_vars/all.yml like:
cifmw_architecture_repo_version_pin: "HEAD"
(or some other name, this is just an example)
then in the roles/kustomize_deploy/defaults/main.yml file - define:
cifmw_kustomize_deploy_architecture_repo_version: "{{ cifmw_architecture_repo_version_pin }}"
(or maybe even
cifmw_kustomize_deploy_architecture_repo_version: "{{ cifmw_architecture_repo_version_pin | default('HEAD') }}"
? Though maybe we shouldn't silently continue and fall back to a different version than we're expecting to be set from group_vars?)
Though this is more verbose, it clearly explains where the var's value is coming from. Rather than expecting readers to keep group vars and Ansible variable precedence in mind when trying to understand where the actual value of a var is coming from, you can clearly see that the default var's value is subject to change according to a definition that happens somewhere else in the repo.
This file in conjuction with:
Allows ci-framework to clone those repos (architecture was already enabled)
when repositories are not already there due to zuul has not marked them as
required-projects or because we're running them outside the zuul ecosystem.
This is the almost the lowest in precedence, so if we still have these vars defined at zuul
level or defaults-vars (passed as extra-vars) or scenarios files, this file is not
taken into consideration.
Precedence chain:
* Zuul repos
* cifmw_reproducer_repositories
* cifmw_reproducer_default_repositories
* Git pull logic in each of the roles
Signed-off-by: Enrique Vallespi Gil evallesp@redhat.com