reorganize var defaults

[KubeKyrie]

What type of PR is this?
/kind flake

What this PR does / why we need it:
reorganize var defaults, and have a default value defined only once
Which issue(s) this PR fixes:

#11822

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Reorganize variable defaults

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: KubeKyrie
Once this PR has been reviewed and has the lgtm label, please assign ant31 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Hi @KubeKyrie. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[VannTen]

Unless this is taking care of all the instances ( I don't think so ? ) please remove the `fixes` to avoid auto closing the issue. Also, I don't think we should have null checksum, we should rather figure out how to obtain the correct ones. /ok-to-test

[ant31]

would it fails if cert_management is undefined ?
If yes that would change the behavior of using script by default when undefined

[VannTen]

Yeah, this would need a default value for cert_management in kubespray-defaults

[ant31]

when this role is executed, nothing say that volume_cross_zone_attachment is defined.
this variable eventually is defined but from another role/inventory.
We shouldn't remove it there

[VannTen]

Default variables are lazily interpreted though, aren't they ? (-> meaning this will work if the value is defined when the variable is used)

[ant31]

Nothing tells you it will be defined for sure.
Maybe it's defined in the openstack role that is not included when not using openstack,
Maybe the playbook ran with --tags X,Y and the role where "volume_cross_zone_attachment" is not executed.

[VannTen]

We're mostly doing role imports rather than include, though, don't we ?

If I'm not mistaken, the tags don't affect roles variables, only the tasks (unless using include, which is dynamic).

However, in any case, I agree it's better than a variable dependent on another should live nearby, ideally in the same file. ~~

Actually, this variable is not used anywhere since 437026f (AFAICS), so we should just delete it :

$ grep -R openstack_blockstorage_ignore roles/
roles/kubespray-defaults/defaults/main/main.yml:openstack_blockstorage_ignore_volume_az: "{{ volume_cross_zone_attachment | default('false') }}"

[ant31]

@VannTen yes, but not all runs imports all roles.

Short test:

role A define "var_a: "I'm var a"
role B define "var_ab: "I'm b and {{var_a}}"

In the test playbook, the install kubernetes nodes run will work (even if role a is not executed) , but not the install control plane.

---
- name: Install Kubernetes nodes
  hosts: localhost
  gather_facts: true
  roles:
    - { role: a, tags: ['a'], when: false}
    - { role: b, tags: ['b'] }


- name: Install the control plane
  hosts: localhost
  gather_facts: true
  roles:
    - { role: b, tags: ['b'] }

Selecting subset of roles is used a lot in kubespray (see cluster.yml)

[ant31]

Hi thank you for the PR.

But I think the default are good and shouldn't be remove as it change behavior and also increase dependencies between roles.

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.