Stop network_plugin/cni dependency when include_role comes from reset role

[jwitko]

What type of PR is this?
/kind bug

What this PR does / why we need it:
When removing a node there is no need to execute the network_plugin/cni dependency and doing so causes an error

Which issue(s) this PR fixes:

Fixes #9549

Special notes for your reviewer:
Running remove-node.yml does an include_role: on network_plugin/<your-network-plugin> with a tasks_from: reset. All of the potential roles that can be loaded include a dependency for network_plugin/cni which will fail because the download role is not included in this playbook (nor should it be).

This PR skips the dependency when the include_role comes from remove-node.yml

Does this PR introduce a user-facing change?:

None

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jwitko / name: Jason Witkowski (28cc719)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jwitko
Once this PR has been reviewed and has the lgtm label, please assign miouge1 for approval by writing /assign @miouge1 in a comment. 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]

Welcome @jwitko!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @jwitko. 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.

[yankay]

/ok-to-test

[yankay]

Thanks @jwitko

I think the dependency can be removed, How do you think about #9563, it can also resolve the issue.

[jwitko]

@yankay I think removing it is definitely the right way to go if thats how you want to proceed. I was trying to be as least impactful to the process to maximize chances of PR merge, but happy to see it removed all-together. My only question would be:
Are there any CNIs that require this dependency? It seems to me like this issue would bite every one of them?

Edit: Unless this is used in initial node provisioning? Then it would be responsible for moving files for install and creating a CNI directory. If the network_plugins/cni role isn't called directly on its own then I would suggest the alternative approach in my PR

[yankay]

@yankay I think removing it is definitely the right way to go if thats how you want to proceed. I was trying to be as least impactful to the process to maximize chances of PR merge, but happy to see it removed all-together. My only question would be: Are there any CNIs that require this dependency? It seems to me like this issue would bite every one of them?

Edit: Unless this is used in initial node provisioning? Then it would be responsible for moving files for install and creating a CNI directory. If the network_plugins/cni role isn't called directly on its own then I would suggest the alternative approach in my PR

Hi @jwitko, Thank you for the advice :-)

The dependency is useful before the #9367, but it's not needed now.

The CNI dependency should be removed from the Cilium and all the other CNI like flannel and weave. And the calico has no dependency and works well. The #9563 try to remove the CNI dependency in cilium first. If it's OK, the other CNI could be removed CNI dependency in the same way.

I think both ways can solve the issue. Remove the CNI dependency could make the code simpler :-)

Thanks again.

[k8s-ci-robot]

@jwitko: 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/test-infra repository.

[jwitko]

@yankay Since your PR was merged I should close this one right? Also... Your PR only handled this for cilium. Are we not concerned with all other CNIs ?

[yankay]

@yankay Since your PR was merged I should close this one right? Also... Your PR only handled this for cilium. Are we not concerned with all other CNIs ?

HI @jwitko
I think we can close this one :-)
Welcome to provide PR to the other CNI. Thank you very much