-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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.
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
Rename ansible groups to use _ instead of - #7552
Rename ansible groups to use _ instead of - #7552
Conversation
Hi @cristicalin. 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 Once the patch is verified, the new status will be reflected by the 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. |
@cristicalin I've remove the "do not merge yet" and put the PR as draft, it's cleaner ;) |
8211a99
to
e725348
Compare
I added the missing compatibility layer so this should be ready for CI and review. |
I'm not sure the failure currently in CI is related to the change, as far as I can see the cluster is up and was able to run workloads. |
/ok-to-test |
d409c44
to
174ae45
Compare
I believe the CI needs some love again since the failing test appears to be an ssh timeout unrelated to the changes in this PR. |
k8s-cluster -> k8s_cluster k8s-node -> k8s_node calico-rr -> calico_rr no-floating -> no_floating Note: kube-node,k8s-cluster groups in upgrade CI need clean-up after v2.16 is tagged
174ae45
to
3fbe32a
Compare
The CI is upset again for some networking (as far as I can tell) reason. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cristicalin, floryut 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:
Approvers can indicate their approval by writing |
@cristicalin Thank you for that 👍 |
* rename ansible groups to use _ instead of - k8s-cluster -> k8s_cluster k8s-node -> k8s_node calico-rr -> calico_rr no-floating -> no_floating Note: kube-node,k8s-cluster groups in upgrade CI need clean-up after v2.16 is tagged * ensure old groups are mapped to the new ones
Looks like this has also changed kube-node to kube_node, though it is not mentioned in the 2.16 release notes or the description of this PR. Actually the description says k8s-node -> k8s_node but those group names do not exist. |
Oops .. you're totally right, I've edit the PR desc in case anyone drop on this PR. |
* rename ansible groups to use _ instead of - k8s-cluster -> k8s_cluster k8s-node -> k8s_node calico-rr -> calico_rr no-floating -> no_floating Note: kube-node,k8s-cluster groups in upgrade CI need clean-up after v2.16 is tagged * ensure old groups are mapped to the new ones
rename ansible groups to use
_
instead of-
k8s-cluster -> k8s_cluster
kube-node -> kube_node
calico-rr -> calico_rr
no-floating -> no_floating
What type of PR is this?
What this PR does / why we need it:
This PR renames some ansible groups to be consistent with dynamic ansible inventory plugins like AWX.
Which issue(s) this PR fixes:
Fixes #7536
Special notes for your reviewer:
This PR touches a lot of files, I apologize for the giant size.
Does this PR introduce a user-facing change?: