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.

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

Speed up old docker package removal #4408

Merged

Conversation

chadswen
Copy link
Member

Both the yum and apt modules support a list as input, this allows us avoid the slower with_items approach, which can take a long time with a large count of cluster nodes.

Both the `yum` and `apt` modules support a list as input, this allows us avoid the slower `with_items` approach, which can take a long time with a large count of cluster nodes.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 29, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chadswen

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 29, 2019
@chadswen
Copy link
Member Author

ci check this

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 29, 2019
@chadswen
Copy link
Member Author

TASK [container-engine/docker : Ensure old versions of Docker are not installed. | RedHat] ***
task path: /kargo-ci/kubernetes-sigs-kubespray/roles/container-engine/docker/tasks/pre-upgrade.yml:10
Friday 29 March 2019  22:26:21 +0000 (0:00:00.271)       0:03:45.423 ********** 
ok: [k8s-54323809-187109797-1] => {"changed": false, "msg": "", "rc": 0, "results": ["docker is not installed", "docker-common is not installed", "docker-engine is not installed", "docker-selinux.noarch is not installed", "docker-client is not installed", "docker-client-latest is not installed", "docker-latest is not installed", "docker-latest-logrotate is not installed", "docker-logrotate is not installed", "docker-engine-selinux.noarch is not installed"]}
ok: [k8s-54323809-187109797-3] => {"changed": false, "msg": "", "rc": 0, "results": ["docker is not installed", "docker-common is not installed", "docker-engine is not installed", "docker-selinux.noarch is not installed", "docker-client is not installed", "docker-client-latest is not installed", "docker-latest is not installed", "docker-latest-logrotate is not installed", "docker-logrotate is not installed", "docker-engine-selinux.noarch is not installed"]}
ok: [k8s-54323809-187109797-2] => {"changed": false, "msg": "", "rc": 0, "results": ["docker is not installed", "docker-common is not installed", "docker-engine is not installed", "docker-selinux.noarch is not installed", "docker-client is not installed", "docker-client-latest is not installed", "docker-latest is not installed", "docker-latest-logrotate is not installed", "docker-logrotate is not installed", "docker-engine-selinux.noarch is not installed"]}

Copy link
Contributor

@mirwan mirwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess fedora with dnf and Suse with zypper are missing here

@chadswen
Copy link
Member Author

chadswen commented Apr 1, 2019

I guess fedora with dnf and Suse with zypper are missing here

@mirwan I didn't change the conditionals. If those weren't handled already that should probably be a separate PR as we were only handling Debian and RedHat ansible_os_family in this task file. I wouldn't know enough about the old package names on other OSes to include it along with this.

@chadswen chadswen requested a review from mirwan April 1, 2019 19:16
@mirwan
Copy link
Contributor

mirwan commented Apr 1, 2019

@mirwan I didn't change the conditionals. If those weren't handled already that should probably be a separate PR as we were only handling Debian and RedHat ansible_os_family in this task file. I wouldn't know enough about the old package names on other OSes to include it along with this.

Arff I missed the unchanged conditions.

@mirwan
Copy link
Contributor

mirwan commented Apr 1, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 1, 2019
@k8s-ci-robot k8s-ci-robot merged commit 5d5c9ca into kubernetes-sigs:master Apr 1, 2019
@chadswen chadswen deleted the speed-up-docker-package-remove branch April 1, 2019 22:09
b23prodtm added a commit to b23prodtm/kubespray that referenced this pull request Apr 7, 2019
* fix(contrib/metallb): adds missing become: true in role (kubernetes-sigs#4356)

On CoreOS, without this, it fails to kubectl apply MetalLB due to lack of privileges.

* Fix kubernetes-sigs#4237: update kube cert path (kubernetes-sigs#4354)

* Use sample inventory file in doc (kubernetes-sigs#4052)

* Revert "Fix kubernetes-sigs#4237: update kube cert path (kubernetes-sigs#4354)" (kubernetes-sigs#4369)

This reverts commit ea7a6f1.

This change modified the certs dir for Kubernetes, but did not move the directories for existing clusters.

* Fix support for ansible 2.7.9 (kubernetes-sigs#4375)

* Use wide for netchecker debug output (kubernetes-sigs#4383)

* Added support of bastion host for reset.yaml (kubernetes-sigs#4359)

* Added support of bastion host for reset.yaml

* Empty commit to triger CI

* Use proxy_env with kubeadm phase commands (kubernetes-sigs#4325)

* clarify that kubespray now supports kubeadm (fixes kubernetes-sigs#4089) (kubernetes-sigs#4366)

* Reduce jinja2 filters in coredns templates (kubernetes-sigs#4390)

* Upgrade to k8s 1.13.5

* Increase CPU flavor for CI (kubernetes-sigs#4389)

* Fix CA cert environment variable for ectd v3 (kubernetes-sigs#4381)

* Added livenessProbe for local nginx apiserver proxy liveness probe (kubernetes-sigs#4222)

* Added configurable local apiserver proxy liveness probe

* Enable API LB healthcheck by default

* Fix template spacing and moved healthz location to nginx http section

* Fix healthcheck listen address to allow kubelet request healthcheck

* Default values for variable dns_servers and dns_domain  are set in two files: (kubernetes-sigs#3999)

values from inventory in roles/kubespray-defaults/defaults/main.yml
hardcoded values in roles/container-engine/defaults/main.yml

dns_servers set empty in roles/container-engine/defaults/main.yml and skydns_server not set in docker_dns_servers variables
also set default value for manual_dns_serve

another variables in roles/container-engine/defaults not need to set

* Fix bootsrap-os role, failing to create remote_tmp (kubernetes-sigs#4384)

* Fix bootsrap-os role, failing to create remote_tmp

* use ansible_remote_tmp hostvar

* Use static files in KubeDNS templating task (kubernetes-sigs#4379)

This commit adapts the "Lay Down KubeDNS Template" task to use the static
files moved by pull request [1]

[1] kubernetes-sigs#4341

* Fix supplementary_addresses rendering error (kubernetes-sigs#4403)

* Corrected cloud name (kubernetes-sigs#4316)

The correct name is Packet, not Packet Host.

* adapt inventory script to python 2.7 version (kubernetes-sigs#4407)

* Calico felix - Fix jinja2 boolean condition (kubernetes-sigs#4348)

* Fix jinja2 boolean condition

* Convert all felix variable to booleans instead.

* Yamllint fixes (kubernetes-sigs#4410)

* Lint everything in the repository with yamllint

* yamllint fixes: syntax fixes only

* yamllint fixes: move comments to play names

* yamllint fixes: indent comments in .gitlab-ci.yml file

* add 1.14.0 checksum, remove 1.11.* checksums (kubernetes-sigs#4401)

* Remove kubedns and dnsmasq. Move dns_late phase after apps (kubernetes-sigs#4406)

Both kubedns and dnsmasq modes are long not maintained.
We should run dns_late steps at the end because sshd
makes DNS lookups during Ansible run and has 2s timeouts
for each failed lookup trying to connect to coredns before
it is ready.

* Speed up old docker package removal (kubernetes-sigs#4408)

Both the `yum` and `apt` modules support a list as input, this allows us avoid the slower `with_items` approach, which can take a long time with a large count of cluster nodes.

* Use install_cni init container for cni copy for calico/canal (kubernetes-sigs#4416)

* Fixed cleanup-docker-orphans.sh to use docker-containerd-shim and containerd-shim (kubernetes-sigs#4418)

* enable kubelet client certificate rotation (kubernetes-sigs#4081)

* enable kubelet client certificate rotation

* change to variable kubelet_rotate_certificates

* remove our config if docker start failed (kubernetes-sigs#4260)

* keep compatibility as it was before (kubernetes-sigs#4268)

* jmespath is required when re-running cluster.yml (kubernetes-sigs#4426)

* Update DNS Autoscaler to 1.4.0 (kubernetes-sigs#4425)

* Update DNS Autoscaler

* Update downloads too

* Fix yamllint

* Fix yamllint

* Update nodelocaldns cache settings (kubernetes-sigs#4423)

* Update CoreDNS to 1.4.0 (kubernetes-sigs#4422)

* Update CoreDNS to 1.4.0

* Update readme to reflect CoreDNS update

* Use docker.io for calico (kubernetes-sigs#4253)

* Add CI for contrib/terraform/ (kubernetes-sigs#4133)

* add Cinder allowVolumeExpansion option (kubernetes-sigs#4415)

* allow Suse OS family (kubernetes-sigs#4430)

* Remove bash-completion (kubernetes-sigs#4431)

* Tell git to ignore .terraform directory (kubernetes-sigs#4428)

The .terraform directory is populated when modules are downloaded:
https://www.terraform.io/docs/commands/get.html
"The modules are downloaded into a local .terraform folder. This folder should not be committed to version control."

* Fix pep8 warnings (kubernetes-sigs#4368)

* Update premoderator to fix Github API throttle (kubernetes-sigs#4424)

* Update premoderator to fix Github API throttle

* Update premoderator script

Add exit codes and document the exit code.

* Fix indentation
b23prodtm added a commit to b23prodtm/kubespray that referenced this pull request Apr 7, 2019
* fix(contrib/metallb): adds missing become: true in role (kubernetes-sigs#4356)

On CoreOS, without this, it fails to kubectl apply MetalLB due to lack of privileges.

* Fix kubernetes-sigs#4237: update kube cert path (kubernetes-sigs#4354)

* Use sample inventory file in doc (kubernetes-sigs#4052)

* Revert "Fix kubernetes-sigs#4237: update kube cert path (kubernetes-sigs#4354)" (kubernetes-sigs#4369)

This reverts commit ea7a6f1.

This change modified the certs dir for Kubernetes, but did not move the directories for existing clusters.

* Fix support for ansible 2.7.9 (kubernetes-sigs#4375)

* Use wide for netchecker debug output (kubernetes-sigs#4383)

* Added support of bastion host for reset.yaml (kubernetes-sigs#4359)

* Added support of bastion host for reset.yaml

* Empty commit to triger CI

* Use proxy_env with kubeadm phase commands (kubernetes-sigs#4325)

* clarify that kubespray now supports kubeadm (fixes kubernetes-sigs#4089) (kubernetes-sigs#4366)

* Reduce jinja2 filters in coredns templates (kubernetes-sigs#4390)

* Upgrade to k8s 1.13.5

* Increase CPU flavor for CI (kubernetes-sigs#4389)

* Fix CA cert environment variable for ectd v3 (kubernetes-sigs#4381)

* Added livenessProbe for local nginx apiserver proxy liveness probe (kubernetes-sigs#4222)

* Added configurable local apiserver proxy liveness probe

* Enable API LB healthcheck by default

* Fix template spacing and moved healthz location to nginx http section

* Fix healthcheck listen address to allow kubelet request healthcheck

* Default values for variable dns_servers and dns_domain  are set in two files: (kubernetes-sigs#3999)

values from inventory in roles/kubespray-defaults/defaults/main.yml
hardcoded values in roles/container-engine/defaults/main.yml

dns_servers set empty in roles/container-engine/defaults/main.yml and skydns_server not set in docker_dns_servers variables
also set default value for manual_dns_serve

another variables in roles/container-engine/defaults not need to set

* Fix bootsrap-os role, failing to create remote_tmp (kubernetes-sigs#4384)

* Fix bootsrap-os role, failing to create remote_tmp

* use ansible_remote_tmp hostvar

* Use static files in KubeDNS templating task (kubernetes-sigs#4379)

This commit adapts the "Lay Down KubeDNS Template" task to use the static
files moved by pull request [1]

[1] kubernetes-sigs#4341

* Fix supplementary_addresses rendering error (kubernetes-sigs#4403)

* Corrected cloud name (kubernetes-sigs#4316)

The correct name is Packet, not Packet Host.

* adapt inventory script to python 2.7 version (kubernetes-sigs#4407)

* Calico felix - Fix jinja2 boolean condition (kubernetes-sigs#4348)

* Fix jinja2 boolean condition

* Convert all felix variable to booleans instead.

* Yamllint fixes (kubernetes-sigs#4410)

* Lint everything in the repository with yamllint

* yamllint fixes: syntax fixes only

* yamllint fixes: move comments to play names

* yamllint fixes: indent comments in .gitlab-ci.yml file

* add 1.14.0 checksum, remove 1.11.* checksums (kubernetes-sigs#4401)

* Remove kubedns and dnsmasq. Move dns_late phase after apps (kubernetes-sigs#4406)

Both kubedns and dnsmasq modes are long not maintained.
We should run dns_late steps at the end because sshd
makes DNS lookups during Ansible run and has 2s timeouts
for each failed lookup trying to connect to coredns before
it is ready.

* Speed up old docker package removal (kubernetes-sigs#4408)

Both the `yum` and `apt` modules support a list as input, this allows us avoid the slower `with_items` approach, which can take a long time with a large count of cluster nodes.

* Use install_cni init container for cni copy for calico/canal (kubernetes-sigs#4416)

* Fixed cleanup-docker-orphans.sh to use docker-containerd-shim and containerd-shim (kubernetes-sigs#4418)

* enable kubelet client certificate rotation (kubernetes-sigs#4081)

* enable kubelet client certificate rotation

* change to variable kubelet_rotate_certificates

* remove our config if docker start failed (kubernetes-sigs#4260)

* keep compatibility as it was before (kubernetes-sigs#4268)

* jmespath is required when re-running cluster.yml (kubernetes-sigs#4426)

* Update DNS Autoscaler to 1.4.0 (kubernetes-sigs#4425)

* Update DNS Autoscaler

* Update downloads too

* Fix yamllint

* Fix yamllint

* Update nodelocaldns cache settings (kubernetes-sigs#4423)

* Update CoreDNS to 1.4.0 (kubernetes-sigs#4422)

* Update CoreDNS to 1.4.0

* Update readme to reflect CoreDNS update

* Use docker.io for calico (kubernetes-sigs#4253)

* Add CI for contrib/terraform/ (kubernetes-sigs#4133)

* add Cinder allowVolumeExpansion option (kubernetes-sigs#4415)

* allow Suse OS family (kubernetes-sigs#4430)

* Remove bash-completion (kubernetes-sigs#4431)

* Tell git to ignore .terraform directory (kubernetes-sigs#4428)

The .terraform directory is populated when modules are downloaded:
https://www.terraform.io/docs/commands/get.html
"The modules are downloaded into a local .terraform folder. This folder should not be committed to version control."

* Fix pep8 warnings (kubernetes-sigs#4368)

* Update premoderator to fix Github API throttle (kubernetes-sigs#4424)

* Update premoderator to fix Github API throttle

* Update premoderator script

Add exit codes and document the exit code.

* Fix indentation

* Upgrade to Helm 2.13.1 (kubernetes-sigs#4445)
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Apr 9, 2019
Both the `yum` and `apt` modules support a list as input, this allows us avoid the slower `with_items` approach, which can take a long time with a large count of cluster nodes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants