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

Add drain logic for upgrade-controller SUC plans #28

Merged
merged 5 commits into from
Jul 30, 2024

Conversation

ipetrov117
Copy link
Contributor

@ipetrov117 ipetrov117 commented Jul 30, 2024

As discussed previously the upgrade-controller should:

  1. Drain the control-plane and worker nodes for both K8s and OS upgrades
  2. Offer the possibility to disable node drain for control-plane/worker nodes or disable the whole node drain procedure of the upgrade-controller

By default the upgrade-controller will execute drain with the following flags:

  • --delete-emptydir-data - continue drain even if there are pods utilising emptyDir storage
  • --timeout 15m0s - the amount of time for which to wait for the drain operation to finish. Ensures that the drain operation will not block the upgrade flow
  • --ignore-daemonsets - ignores DaemonSet-managed pods
  • --force - continue even if there are pods that do not have a controller (single pods)

Due to the nature of how we do upgrades, node drain will happen twice - once during OS upgrades and once during K8s upgrades.

Tested scenarios:

  • Full node drain on a single cluster
  • Full node drain on a HA cluster
  • Worker only node drain on a HA cluster
  • Control-plane only node drain on a HA cluster

Copy link
Contributor

@atanasdinov atanasdinov left a comment

Choose a reason for hiding this comment

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

Looks great! I left two minor comments - let me know what you think.

@@ -48,6 +48,15 @@ type UpgradePlanSpec struct {
// ReleaseVersion specifies the target version for platform upgrade.
// The version format is X.Y.Z, for example "3.0.2".
ReleaseVersion string `json:"releaseVersion"`
// +optional
Drain *Drain `json:"drain"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put a comment describing this in order for the CRD to also include it?


type Drain struct {
// +optional
ControlPlanes *bool `json:"controlPlanes"`
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a nitpick but I believe using a singular form here controlPlane is the better wording. There are multiple control plane nodes in a cluster, but not multiple control planes.

@ipetrov117 ipetrov117 merged commit 592c497 into suse-edge:main Jul 30, 2024
2 checks passed
@ipetrov117 ipetrov117 deleted the drain-logic-impl branch August 9, 2024 07:00
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.

2 participants