Skip to content
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.

Sign up for GitHub

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

Jump to bottom

increase maximal_ansible_version to 2.12.0 #7887

Closed
wants to merge 178 commits into from
Closed

increase maximal_ansible_version to 2.12.0 #7887

wants to merge 178 commits into from

Conversation

mmelyp
Copy link

@mmelyp mmelyp commented Aug 17, 2021

No description provided.

@k8s-ci-robot
Copy link
Contributor

Welcome @mmelyp!

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
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 17, 2021
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 17, 2021
@k8s-ci-robot k8s-ci-robot requested review from bozzo and oomichi August 17, 2021 19:53
@mmelyp mmelyp changed the title increase maximal_ansible_version to 2.11.4 (#7884) increase maximal_ansible_version to 2.11.4 Aug 17, 2021
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 17, 2021
@champtar
Copy link
Contributor

Why not make it 2.12.0 so we don't have to constantly bump version ?

@mmelyp
Copy link
Author

mmelyp commented Aug 17, 2021

Why not make it 2.12.0 so we don't have to constantly bump version ?

Yes makes sense better to 2.12.0. Thank you!

@mmelyp mmelyp changed the title increase maximal_ansible_version to 2.11.4 increase maximal_ansible_version to 2.12.0 Aug 17, 2021
@cristicalin
Copy link
Contributor

Have you tested ansible 2.11.x with kubespray? The transition to 2.10 took quite a lot of work to get running and I would pleasantly amazed if 2.11 would work out of the box.

@rtsp
Copy link
Member

rtsp commented Aug 27, 2021

Have you tested ansible 2.11.x with kubespray? The transition to 2.10 took quite a lot of work to get running and I would pleasantly amazed if 2.11 would work out of the box.

I personally used 2.11.3 for a while and it is fine for as far as I can tell.

You can also run Kubespray with -e maximal_ansible_version=2.12.0 as a workaround while this PR is waiting for test & merge.

@fungusakafungus
Copy link
Contributor

There's at least a problem with mitogen not supporting 2.11.

@redtex
Copy link

redtex commented Sep 8, 2021

There's at least a problem with mitogen not supporting 2.11.

Mitrogen has 0.3.0-RC0 with ansible 2.10+ support, so soon it will be not an issue

@oomichi oomichi mentioned this pull request Sep 15, 2021
Closed
@trollqaa
Copy link

Mitrogen has 0.3.0-RC0 with ansible 2.10+ support, so soon it will be not an issue

yes, but 2.11 not supported and "fixes" in PR is bad.
I tried to make edits in accordance with the comments in the PR, but at the output I got a completely non-working mitogen, and the author is in no hurry to update. He stated that he stopped using ansible and is now of little interest to him.

@redtex
Copy link

redtex commented Oct 11, 2021

Mitrogen has 0.3.0-RC0 with ansible 2.10+ support, so soon it will be not an issue

yes, but 2.11 not supported and "fixes" in PR is bad. I tried to make edits in accordance with the comments in the PR, but at the output I got a completely non-working mitogen, and the author is in no hurry to update. He stated that he stopped using ansible and is now of little interest to him.

Well, it means, that Mitrogen has no more actual support by developer, and can't be viewed as stopping factor.

@floryut
Copy link
Member

floryut commented Oct 11, 2021 •
edited
Loading

At that rate (a year without release, no more support for new Ansible releases) I wonder if we should even keep Mitogen support in Kubespray.

@cristicalin
Copy link
Contributor

We already carry a few workarounds for mitogen without any clear benefit. It would simplify our maintenance burden to get rid of it.

@trollqaa
Copy link

Without Mitogen, everything is very poor in terms of the speed of performing roles in Ansible, especially if there are a lot of servers. Now, if there was any alternative to it, it would be gorgeous. Or replace with something Ansible ...
In its current form (without Mitogen), everything is sad, and I want to cry (:D)

@cristicalin
Copy link
Contributor

@trollqaa do you have any concrete numbers in regards to kubespray deployment speed with and without mitogen? I'm ready to nuke it from orbit since my own testing has proven for clusters with < 50 nodes to not give such a considerable advantage when applying settings like ssh pipelining (https://docs.ansible.com/ansible/latest/reference_appendices/config.html#ansible-pipelining)

@fungusakafungus
Copy link
Contributor

Another argument against mitogen: #8125

@cristicalin cristicalin mentioned this pull request Oct 31, 2021
Merged
@fungusakafungus
Copy link
Contributor

do you have any concrete numbers in regards to kubespray deployment speed with and without mitogen? I'm ready to nuke it from orbit since my own testing has proven for clusters with < 50 nodes to not give such a considerable advantage when applying settings like ssh pipelining (https://docs.ansible.com/ansible/latest/reference_appendices/config.html#ansible-pipelining)

I just tested a scale.yml run on a 53 nodes cluster: 00:37:24 with mitogen, 00:29:31 without mitogen and with default ssh settings

cristicalin and others added 14 commits November 16, 2021 00:16
@cristicalin @mmelyp
Docker: replace the use of containerd_version with docker_containerd_…
28bd133 
…version to avoid causing conflicts when bumping containerd_version (#8130)
@zhengtianbao @mmelyp
Add krew auto completion (#8171)
53fccc3
@cristicalin @mmelyp
containerd: upgrade versions 1.4.11 and 1.5.7 and make 1.4.11 the def…
9c5bd54 
…ault (#8129)
@cristicalin @mmelyp
nodelocaldns: allow a secondary pod for nodelocaldns for local-HA (#8100
958b77b 
)

* nodelocaldns: allow a secondary pod for nodelocaldns for local-HA

* CI: add job to test nodelocaldns secondary
@cristicalin @mmelyp
Kata-Containers: Fix kata-containers runtime (#8068)
fc954d7 
* Kata-containes: Fix for ubuntu and centos sometimes kata containers fail to start because of access errors to /dev/vhost-vsock and /dev/vhost-net

* Kata-containers: use similar testing strategy as gvisor

* Kata-Containers: adjust values for 2.2.0 defaults

Make CI tests actually pass

* Kata-Containers: bump to 2.2.2 to fix sandbox_cgroup_only issue
@cristicalin @mmelyp
Exercise multiple ansible versions in CI (#8172)
85fc75c 
* Ansible: separate requirements files for supported ansible versions

* Ansible: allow using ansible 2.11

* CI: Exercise Ansible 2.9 and Ansible 2.11 in a basic AIO CI job

* CI: Allow running a reset test outside of idempotency tests and running it in stage1

* CI: move ubuntu18-calico-aio job to stage2 and relay only on ubuntu20 with the variously supported ansible versions for stage1

* CI: add capability to install collections or roles from ansible-galaxy to mitigate missing behavior in older ansible versions
@Ajarmar @mmelyp
Added tolerations for cinder-csi-nodeplugin DaemonSet (#8137)
84da769
@llarsson @mmelyp
Fixes various issues in vSphere Terraform code (#8178)
849dade 
* Fixes various issues in vSphere Terraform code

Provided to address various shortcomings and to fix the following
issue in upstream Kubespray:

#8176

* Resolves Terraform formatting issues

* Sets default prefix to human-readable name

* Documents new default prefix in README
@zhengtianbao @mmelyp
Fix krew auto completion command not found at lower version (#8185)
bce6b25
@zhengtianbao @mmelyp
calico add support for container ip forwarding setting (#8184)
ef192d6
@liupeng0518 @mmelyp
change krew uri to krew_download_url (#8190)
cda7d05
@EDGsheryl @mmelyp
Remove extra parameter kube_proxy_remove (#8158)
58a919b 
Signed-off-by: EDGsheryl <edgsheryl@gmail.com>
@cristicalin @mmelyp
Python: bring back python 2.7 support for ansible 2.9 in supported EL…
21869f0 
… distributions (#8192)
@zhengtianbao @mmelyp
Remove helm duplicate check (#8196)
a6c671b
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 15, 2021
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 15, 2021 •
edited
Loading

CLA Not Signed

@mmelyp mmelyp closed this Nov 15, 2021
@mmelyp mmelyp deleted the #7884 branch November 15, 2021 23:33
@mmelyp mmelyp restored the #7884 branch November 15, 2021 23:36
@mmelyp mmelyp deleted the #7884 branch November 15, 2021 23:36
@mmelyp mmelyp restored the #7884 branch November 15, 2021 23:38
@mmelyp mmelyp reopened this Nov 15, 2021
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 15, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mmelyp

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

Needs approval from an approver in each of these files:

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

@cristicalin
Copy link
Contributor

This can be safely closed since we implemented support for ansible 2.11 and are now validating it in CI.

/close

@k8s-ci-robot
Copy link
Contributor

@cristicalin: Closed this PR.

In response to this:

This can be safely closed since we implemented support for ansible 2.11 and are now validating it in CI.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bozzo bozzo Awaiting requested review from bozzo

@oomichi oomichi Awaiting requested review from oomichi

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.