The description of cluster paused and skipping remediation of MHC should be clearer #9026

haijianyang · 2023-07-21T05:48:54Z

What steps did you take and what happened?

Original link https://cluster-api.sigs.k8s.io/tasks/automated-machine-management/healthchecking.html#skipping-remediation

What did you expect to happen?

Original description:
`Implicit skipping when the resource is paused (using cluster.x-k8s.io/paused annotation):

When a cluster is paused, none of the machines in that cluster are considered for remediation.
When a machine is paused, only that machine is not considered for remediation.
A cluster or a machine is usually paused automatically by Cluster API when it detects a migration.`

MHC Controller

// Return early if the object or Cluster is paused.
if annotations.IsPaused(cluster, m) {
	log.Info("Reconciliation is paused for this object")
	return ctrl.Result{}, nil
}

// IsPaused returns true if the Cluster is paused or the object has the `paused` annotation.
func IsPaused(cluster *clusterv1.Cluster, o metav1.Object) bool {
	if cluster.Spec.Paused {
		return true
	}
	return HasPaused(o)
}

According to the MHC code, only when cluster.Spec.Paused is true, none of the machines in that cluster are considered for remediation.

Cluster with cluster.x-k8s.io/paused annotation will only pause the reconciliation of the cluster, not the reconciliation of other resources including MHC.

Cluster API version

v1.4.4

Kubernetes version

No response

Anything else you would like to add?

No response

Label(s) to be applied

/kind bug
One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

killianmuldoon · 2023-07-21T09:56:06Z

/triage accepted

You're right that this is confusing - would be a good idea to clean up the documentation around this.

/help

k8s-ci-robot · 2023-07-21T09:56:08Z

@killianmuldoon:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

Why are we solving this issue?
To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
Does this issue have zero to low barrier of entry?
How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/triage accepted

You're right that this is confusing - would be a good idea to clean up the documentation around this.

/help

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.

SubhasmitaSw · 2023-07-25T04:49:11Z

/assign

SubhasmitaSw · 2023-07-25T05:08:10Z

@killianmuldoon the controller spec definition has to be modified or just the documentation needs more clarity?

killianmuldoon · 2023-07-25T07:59:16Z

For this we just need to clear up the documentation.

SubhasmitaSw · 2023-07-25T10:35:00Z

cool, thanks @killianmuldoon

antonblr · 2023-12-06T18:21:24Z

Should IsPaused check for Cluster annotation as well? It is checking for cluster.spec.paused, but it is not even documented. Only annotations are documented, and MHC explicitly calling func named annotations.IsPaused func, not spec.IsPaused, hinting that it will check the annotations.

Something like:

// util/annotations/helpers.go

// IsPaused returns true if the Cluster or object is paused.
func IsPaused(cluster *clusterv1.Cluster, o metav1.Object) bool {
	if cluster.Spec.Paused {
		return true
	}

        // Cluster or object  has the `paused` annotation
	return HasPaused(cluster) || HasPaused(o)
}

sbueringer · 2023-12-08T13:51:25Z

Not sure if we should introduce a second way to achieve the same thing (i.e. pausing a Cluster). If there is documentation missing we can update the documentation instead of changing the implementation.

fabriziopandini · 2024-04-11T18:27:08Z

/priority backlog

enxebre · 2024-05-29T13:13:40Z

@fabriziopandini shall we close this in favour of #10130?

fabriziopandini · 2024-05-30T15:21:49Z

According to the MHC code, only when cluster.Spec.Paused is true, none of the machines in that cluster are considered for remediation.

AFAIK this is not true, or at least it is only partially true.

MHC does not remediate any machine in the cluster if the cluster is paused (pausing the cluster is like pausing all the MHC resources belonging to it).
A specific MHC resource does not remediate any target machine if the MHC instance is paused.
Paused machines are not remediated by MHC.

Given that, I suggest improving the note in https://cluster-api.sigs.k8s.io/tasks/automated-machine-management/healthchecking.html#skipping-remediation to

There are scenarios where remediation for a machine may be undesirable (eg. during cluster migration using clusterctl move). For such cases, MachineHealthCheck provides the following mechanisms to skip remediation.

- Users can skip remediation for a specific machine by setting the cluster.x-k8s.io/skip-remediation annotation on it.
- Paused Machines (Machines with the cluster.x-k8s.io/paused annotation) are not considered for remediation.
- If a specific MHC resource is paused (using cluster.x-k8s.io/paused annotation), it will stop to remediate the corresponding target machines.
- If the Cluster is paused (using the cluster.x-k8s.io/paused annotation or by setting cluster.spec.paused to true), all the MHC resources belonging to the Cluster will be implicitly paused, and thus stop remediating target machines.

Note: the last option (pausing the Cluster) is the one used by clusterctl move.

/good-first-issue

@fabriziopandini shall we close this in favour of #10130?

This issue is about improving the description of the MHC behaviuor in the book.
The linked issue instead is about surfacing what is paused, so I don't think they overlap.

k8s-ci-robot · 2024-05-30T15:21:52Z

@fabriziopandini:
This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

Why are we solving this issue?
To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
Does this issue have zero to low barrier of entry?
How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

According to the MHC code, only when cluster.Spec.Paused is true, none of the machines in that cluster are considered for remediation.

AFAIK this is not true, or at least it is only partially true.

MHC does not remediate any machine in the cluster if the cluster is paused (pausing the cluster is like pausing all the MHC resources belonging to it).

A specific MHC resource does not remediate any target machine if the MHC instance is paused.

Paused machines are not remediated by MHC.

Given that, I suggest improving the note in https://cluster-api.sigs.k8s.io/tasks/automated-machine-management/healthchecking.html#skipping-remediation to
There are scenarios where remediation for a machine may be undesirable (eg. during cluster migration using clusterctl move). For such cases, MachineHealthCheck provides the following mechanisms to skip remediation.

- Users can skip remediation for a specific machine by setting the cluster.x-k8s.io/skip-remediation annotation on it.
- Paused Machines (Machines with the cluster.x-k8s.io/paused annotation) are not considered for remediation.
- If a specific MHC resource is paused (using cluster.x-k8s.io/paused annotation), it will stop to remediate the corresponding target machines.
- If the Cluster is paused (using the cluster.x-k8s.io/paused annotation or by setting cluster.spec.paused to true), all the MHC resources belonging to the Cluster will be implicitly paused, and thus stop remediating target machines.

Note: the last option (pausing the Cluster) is the one used by clusterctl move.
/good-first-issue

@fabriziopandini shall we close this in favour of #10130?

This issue is about improving the description of the MHC behaviuor in the book.
The linked issue instead is about surfacing what is paused, so I don't think they overlap.

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-sigs/prow repository.

SD-13 · 2024-06-29T17:55:44Z

/assign

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 21, 2023

k8s-ci-robot assigned SubhasmitaSw Jul 25, 2023

SubhasmitaSw mentioned this issue Jul 25, 2023

🐛 clarify description of cluster paused and skipping remediation of MHC #9076

Closed

k8s-ci-robot added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Apr 11, 2024

k8s-ci-robot added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label May 30, 2024

k8s-ci-robot assigned SD-13 Jun 29, 2024

SD-13 mentioned this issue Jun 29, 2024

🐛 Clarify cluster paused and skipping remediation steps #10817

Merged

chrischdi mentioned this issue Jul 4, 2024

🌱 Update implicit skipping remediation for Cluster/MD pause #10827

Closed

k8s-ci-robot closed this as completed in #10817 Jul 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The description of cluster paused and skipping remediation of MHC should be clearer #9026

The description of cluster paused and skipping remediation of MHC should be clearer #9026

haijianyang commented Jul 21, 2023

killianmuldoon commented Jul 21, 2023

k8s-ci-robot commented Jul 21, 2023

SubhasmitaSw commented Jul 25, 2023

SubhasmitaSw commented Jul 25, 2023

killianmuldoon commented Jul 25, 2023

SubhasmitaSw commented Jul 25, 2023

antonblr commented Dec 6, 2023 •

edited

Loading

sbueringer commented Dec 8, 2023

fabriziopandini commented Apr 11, 2024

enxebre commented May 29, 2024

fabriziopandini commented May 30, 2024

k8s-ci-robot commented May 30, 2024

SD-13 commented Jun 29, 2024

The description of cluster paused and skipping remediation of MHC should be clearer #9026

The description of cluster paused and skipping remediation of MHC should be clearer #9026

Comments

haijianyang commented Jul 21, 2023

What steps did you take and what happened?

What did you expect to happen?

Cluster API version

Kubernetes version

Anything else you would like to add?

Label(s) to be applied

killianmuldoon commented Jul 21, 2023

k8s-ci-robot commented Jul 21, 2023

Guidelines

SubhasmitaSw commented Jul 25, 2023

SubhasmitaSw commented Jul 25, 2023

killianmuldoon commented Jul 25, 2023

SubhasmitaSw commented Jul 25, 2023

antonblr commented Dec 6, 2023 • edited Loading

sbueringer commented Dec 8, 2023

fabriziopandini commented Apr 11, 2024

enxebre commented May 29, 2024

fabriziopandini commented May 30, 2024

k8s-ci-robot commented May 30, 2024

Guidelines

SD-13 commented Jun 29, 2024

antonblr commented Dec 6, 2023 •

edited

Loading