Improve validation in Cluster.spec.topology

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle

https://cluster-api.sigs.k8s.io

Apache License 2.0

3.48k stars 1.29k forks source link

Improve validation in Cluster.spec.topology #8721

Open sbueringer opened 1 year ago

sbueringer commented 1 year ago

What would you like to be added (User Story)?

I think it would be good if we can go over Cluster.spec.topology and check if we have validation for all fields where it is appropriate.

Some examples:

MachineHealthCheckTopology: we should have validation that if enable is set to false no MachineHealthCheckClass fields should be set
MachineDeploymentStrategy: fields from MachineRollingUpdateDeployment should only be allowed to be set if Type is RollingUpdate
- The same check is missing in the MD webhook

We should in general check if there is any validation that we either have only in Cluster.spec.topology or the corresponding webhook of the core type (MD/MHC) and make sure they are sync.

This way we can avoid that the Cluster topology controller is creating MD/MHC objects which are already invalid based on their webhook

Detailed Description

Anything else you would like to add?

No response

Label(s) to be applied

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

k8s-ci-robot commented 1 year ago

This issue is currently awaiting triage.

If CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

fabriziopandini commented 1 year ago

Note: while enforcing those checks, we should pay attention to not blocking users with misconfigurations (e.g onDelete strategy + rollout strategy set)

sbueringer commented 1 year ago

This would come down to only doing this validation when the fields are newly set and keep allowing invalid values if they are already invalid?

k8s-triage-robot commented 7 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue as fresh with /remove-lifecycle stale
Close this issue with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

chaunceyjiang commented 7 months ago

/remove-lifecycle stale

fabriziopandini commented 4 months ago

/priority important-longterm

k8s-triage-robot commented 1 month ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue as fresh with /remove-lifecycle stale
Close this issue with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

sbueringer commented 1 month ago

/lifecycle frozen

fabriziopandini commented 1 month ago

/help

k8s-ci-robot commented 1 month ago

@fabriziopandini: 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](https://github.com/kubernetes-sigs/cluster-api/issues/8721): >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.