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

Idempotency fixes for etcd certs and resolvconf tasks #1138

Merged
merged 2 commits into from
Mar 16, 2017

Conversation

mattymo
Copy link
Contributor

@mattymo mattymo commented Mar 14, 2017

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 14, 2017
@mattymo
Copy link
Contributor Author

mattymo commented Mar 14, 2017

ci check this

@mattymo
Copy link
Contributor Author

mattymo commented Mar 15, 2017

@bogdando are you free to check this out?

Copy link
Contributor

@holser holser left a comment

Choose a reason for hiding this comment

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

LGTM

backup: yes
follow: yes
with_nested:
- "{{ [resolvconffile] + [base|default('')] + [head|default('')] }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

you may use map extract patter to have more understandable format

- "{{ [ resolvconffile, base|default(''), head|default('') ] | difference(['']) }}"
in this case you will not need to have 'when: item[0] != ""' condition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll follow up with a small patch to fix this

@@ -114,7 +118,9 @@
- name: Gen_certs | Prepare tempfile for unpacking certs
shell: mktemp /tmp/certsXXXXX.tar.gz
register: cert_tempfile

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO

delegate_to: "{{groups['etcd']}}"
when: sync_certs|default(false) and inventory_hostname != groups['etcd'][0]

looks better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't work that way... you have to loop with multiple delegate_to targets (which is slower). And we use the same rule many times.

@mattymo mattymo merged commit 02fed4a into kubernetes-sigs:master Mar 16, 2017
@mattymo mattymo deleted the idempotency-fixes branch April 18, 2017 10:30
hase1128 pushed a commit to hase1128/kubespray that referenced this pull request Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants