kubespray-defaults: Check for boostrap-os FQDN

[VannTen]

What type of PR is this?
/kind bug

What this PR does / why we need it:

When installed as an ansible collection, roles in
ansible_play_role_names will be designated by their FQDN (i.e
'kubernetes-sigs.kubespray.).

It means we need to check for both when checking for roles in the play.

Which issue(s) this PR fixes:

Fixes #10432

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

kubespray-defaults: Check for boostrap-os FQDN

@RikuXan can you test that fix ?

[k8s-ci-robot]

Hi @VannTen. 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/test-infra repository.

[RikuXan]

I'll probably not get around to actually testing it in my environment until next week, but if I'm not mistaken, the proposed change would always add the two tasks in question since one of the checks around the or would always fail, either when kubespray is used as a collection (first condition) or when it's not (second condition).

I think what could work would be prepending the role name with ansible_collection_name and a dot if the variable is set/non-empty. This would also have the benefit of working still, even if the namespace or collection were to be changed in the future (Edit: just saw that you already added the variable, 👍). I'm just not sure what would be an elegant way of expressing this in Ansible. Maybe something like (ansible_collection_name + '.' if ansible_collection_name else '') + 'bootstrap-os' not in ansible_play_role_names, there might be a more concise way to write it, but I think this should generate the correct role name in both cases.

I am kinda confused that I cannot find any code using these two variables in conjunction anywhere. I would have assumed that someone somewhere would have stood before a similar issue, but either my proposed solution has a massive drawback or the requirement of working as both a collection module as well as a standalone playbook is much rarer than I would've thought.

[VannTen]

On Fri, Nov 03, 2023 at 09:26:50AM -0700, Philipp Holler wrote: the proposed change would always add the two tasks in question since one of the checks around the `or` would always fail, either when kubespray is used as a collection (first condition) or when it's not (second condition).

Well, yeah, one will always fail. But the two possible cases are: - both test fails (neither kubernetes-sigs.kubespray.boostrap-os nor boostrap-os are in the list - one succeeds Which is enough to know whether boostrap-os is in the play or not.

I think what could work would be prepending the role name with `ansible_collection_name` and a dot if the variable is set/non-empty. This would also have the benefit of working still, even if the namespace or collection were to be changed in the future. I'm just not sure what would be an elegant way of expressing this in Ansible.

I tried that way first, but I can't use ansible_collection_name without checking if it's defined first, which results in a quite long condition. Since the collection name is unlikely to change and that's the only place we check that, I'd rather keeps things simple.

…

------ Should we use that pattern more often in the future, I would rather have a normalized roles list: ```yaml ansible_play_role_names_normalized: "{{ ansible_play_role_names | map('split', '.') | map('last') }}" # something like that ```

-- Max Gautier

[yankay]

/ok-to-test

[floryut]

@VannTen Thank you 👍

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, VannTen

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [floryut]

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

[yankay]

Thanks @VannTen
/lgtm