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

Restart services with handlers #52

Merged
merged 11 commits into from
Jul 12, 2019
Merged

Conversation

amimof
Copy link
Owner

@amimof amimof commented Jul 12, 2019

This PR adds Handlers to those roles that have servers that needs to be restarted at the end of each play. Namely:

  • containerd
  • kubelet
  • kube-apiserver
  • kube-scheduler
  • kube-controller-manager

@amimof amimof mentioned this pull request Jul 12, 2019
3 tasks
@anton-johansson
Copy link
Collaborator

This looks really interesting! I would love to test this. However, I can't seem to figure out how this helps running only the restart in serial mode? I guess we still want to perform all the work on all nodes in parallell, but only restart in serial (or 50% serial, or similar).

How would you configure this?

@amimof
Copy link
Owner Author

amimof commented Jul 12, 2019

@anton-johansson It doesn't add serial. We need another PR for that. But adding would be very simple. Add serial to the plays in install.ymlsomething like:

- hosts: localhost
  connection: local
  roles:
  - certificates

- hosts: etcd
  serial: "{{ serial_etcd | default(-1) }}"
  roles:
  - etcd

- hosts: masters
  serial: "{{ serial_masters | default(-1) }}"
  roles:
  - kube-apiserver
  - kube-controller-manager
  - kube-scheduler

- hosts: nodes
  serial: "{{ serial_nodes | default(-1) }}"
  roles:
  - cni
  - containerd
  - runc
  - kube-proxy
  - kubelet

Also, commit 8e290b9 adds a handler for containerd however wait_for only checks if the socket is created and might not be the best way of checking if containerd is up and ready. #47 would solve this i guess.

@amimof
Copy link
Owner Author

amimof commented Jul 12, 2019

@anton-johansson I think you can go ahead and re-open #48 again and rebase once this PR is merged

@anton-johansson
Copy link
Collaborator

anton-johansson commented Jul 12, 2019

Aha, okay, that makes sense. So the plan is to run everything in serial, and not just restarts? That's okay with me, just curious.

I'll reopen #48 and I'll see if I can make the appropriate changes.

@amimof
Copy link
Owner Author

amimof commented Jul 12, 2019

Aha, okay, that makes sense. So the plan is to run everything in serial, and not just restarts? That's okay with me, just curious.

I'll reopen #48!

Well yeah I can't figure out another way since it would mean that restarts are performed in a separate play which would require a lot of checks to determine if stuff has changed in order to perform restarts. Because we only want to perform restarts if something has changed. No point in restarting stuff if the system hasn't been altered. That is what handlers are good at.

@amimof amimof merged commit 1499f86 into master Jul 12, 2019
@amimof amimof deleted the restart_services_with_handlers branch July 12, 2019 09:57
@anton-johansson
Copy link
Collaborator

Okay, that's fair enough for me! Thinking about it, if I want pure high availability, it feels good to actually not touch nodes at all that are waiting for "their turn".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants