Umbrella issue: API changes

[sbueringer]

Goal of this issue is to provide an overview over issues related to API changes.

P0: mandatory for first v1.11 alpha
P1: nice 2 have for first v1.11 alpha

Important

• [P0] Remove DefaulterRemoveUnknownOrOmitableFields from CABPK/ KCP (reintroduced in ⚠️ Partially revert: Remove DefaulterRemoveUnknownOrOmitableFields mutating webhook option #12290)

  • ⚠️ KCP tolerates diff not leading to changes on machines #12402
  • ⚠️ Remove DefaulterRemoveUnknownOrOmitableFields mutating webhook option (again) #12404
  • Bump CAPI in CAPV
  • Follow-up (low prio): move tests currently implemented in the cloudinit package near to the new feature in KCP
  • Follow-up (low prio): stop setting TypeMeta in typed objects (probably mostly in tests)

• [P1] Audit our APIs:

  • Consistency:
    • Audit Machine-level fields across CC, Cluster.spec.topology, MD, MS, KCP and check if we can make it more consistent
    • Audit all strategy fields, including rolloutStrategy / rolloutBefore / rolloutAfter Configurable machine replacement #10946 Improve API around machine deletion strategy #7021 (also future-proof for in-place & for Cluster.nodeDeletionStrategy)
  • Consider to stop embedding structs, e.g. Machine struct directly in MD/MS (e.g. fields like clusterName, providerID don't belong there)
  • Cluster's spec.ControlPlaneEndpoint is marked optional but not a pointer type #6416
    • @sbueringer Also check usages of APIEndpoint and take a look at Change the controlPlaneEndpoint during kubernetes cluster running #5005
  • Look into ClusterClass.spec.controlPlane / infrastructure (also consider the templateless CC use case) (decide if the fields should be optional or mandatory)
    • Related: remove v1beta2 exclude for "field Ref is marked as required, should not be a pointer": ⚠️ Drop unnecessary fields from ClusterClass template references #12358 (comment), Similar for Machine.status.nodeRef

• API field best practices

  • statusoptional: all direct children within status should be optional: 🌱 Enable statusoptional linter #12229
  • nomaps: ⚠️ Enable nomaps linter, Remove unused kubeadm ClusterStatus struct, Migrate Cluster.status.failureDomains to array #12083
  • nodurations: Remove Duration types: ⚠️ Migrate API to use *Seconds instead of metav1.Duration fields #12327
  • ssatags: New Linter: ssatags kube-api-linter#32 (decide case by case because of edge cases with schema transitions) + Current v1beta1 API Types do not define list-type annotation #6504
  • optionalfields: [WIP] 🌱 Enable optionalfields linter #12299
    • Get rid of the findings
      • 🌱 Set minimum=1 on ObservedGeneration and KubeadmConfig APIEndpoint bindPort #12417
      • 📖 Align v1beta2 contract to API conventions around optional fields #12418
      • ⚠️ Use MachineSetDeletePolicy enum in MD & MS API #12419
      • ⚠️ Make Template in MachineSet & Spec in MachineTemplateSpec required #12420
      • ⚠️ Change type of int32 fields with valid zero value to *int32 #12424
      • ⚠️ Change type of *string fields with invalid zero value to string #12429
      • ⚠️ Align Spec fields to optionalfields API conventions #12431
    • Follow-up (low prio): Revisit our own API conventions in our docs
    • Follow-up (next API bump): Revisit if we want to make mor struct fields non-pointers (add omitzero, MinProperties), example: ControlPlaneClass.NamingStrategy

• Consider filtering out "well-known" API warnings in controllers (deprecated API is still used)

Status fields & conditions:

• @fabriziopandini (📖 Proposal: Improving status in CAPI resources #10897)
  • Consider aligning Cluster API conditions to upstream conditions #7635
  • Deprecate KCP.Available condition #10532
  • Improve status.Ready Fields in the contract #10707
  • Mismatch between ControlPlaneReady and Conditions.ControlPlaneReady #7099
  • Add MachineNodeHealthyCondition to Machine’s Ready condition #3974
  • Add conditions to MachineDeployment Object #3486
  • Machine failures not propagated up to MachineDeployment.Status #5635
  • [P1] Consistent reporting of paused resources #10130
  • The ready condition of MachineDeployment doesn't change when a Machine CR is deleted #7461
• out of scope for the first iteration of the proposal, but the same principles will apply
  • Make it easier to understand if a ClusterClass is ready #8137
  • MachineSet should reset MachineReady conditions when scaled down to zero replicas #5453
  • Add more descriptive Message to DrainingFailedReason and DrainingReason #10056
  • Feedback: 🌱 Add BootstrapFailedMachineError error #10360

MachineDeployment:

• [P1] Paused behaviour is inconsistent  #6966:
  • Check behavior of upstream Deployment.spec.paused, see if the same makes sense for us, if yes => suggest to rename the field to disambiguate

KubeadmConfig / KubeadmConfigTemplate / KubeadmControlPlane / KubeadmControlPlaneTemplate:

• Consider getting rid of TypeMeta in InitConfiguration/...

Machine:

• [TBD] Check how bootstrap.ref/datasecret are modelled: BootstrapConfig should be optional for MachinePool based ClusterClass #10943 (comment) & check in all the places where we use MachineSpec (MD/MS/MP/...)
  • Think about use cases where user set dataSecretName: "" (usually because they don't want to use bootstrap)
  • Consider renaming KubeadmConfig/Machine.DataSecretName to *Ref.Name

KubeadmConfig / KubeadmConfigTemplate / KubeadmControlPlane / KubeadmControlPlaneTemplate:

• Consider getting rid of TypeMeta in InitConfiguration/...

MachineHealthCheck:

• Deprecate MachineHealthCheck MaxUnhealthy and UnhealthyRange #10722 [deprecated] [will be removed in v1beta2]
• Improve UX of MachineHealthCheckStatus fields #5291 [not planned]

CRD Columns: (can be already done today)

• [P0] Audit if we broke something when introducing v1beta2
• Fine tuning of print columns
  • rif 📖 Proposal: Improving status in CAPI resources #10897 (review)
  • rif 📖 Proposal: Improving status in CAPI resources #10897 (comment)
  • from Apr 9th office hours, let's consider if to hide phase by default (aligns to K8s guidelines)

• @fabriziopandini (📖 Proposal: Improving status in CAPI resources #10897)
  • Ensure consistency across replica-based CLI printer columns #6238
  • Rename 'replicas' to 'current' in KubeadmControlPlane, MachineDeployments and MachineSet kubectl output #6227
  • API conventions: CRD columns: status vs intent #5332
  • out of scope for the first iteration of the proposal, but the same principles will apply
    • Add more print columns to ClusterResourceSet/Bindings for better tracking #9007
• Consider how we can improve our columns once we can rely on CEL for CRD columns:
  • Consider introducing a class.namespace column on the Cluster object once we can rely on CEL support for CRD columns: 🌱 Provide namespace of the referenced ClusterClass in TEMPLATENAMESPACE #11802 (comment)

ClusterResourceSetBinding

• Consider renaming ClusterResourceSetBinding.ClusterResourceSetName to ClusterResourceSetRef.Name

Backlog (probably won't be done in v1.11 / v1beta2)

ClusterClass & Cluster.spec.topology:

• Enable lifecycle hooks explicitly in the ClusterClass #8211
• Provide explicit definition of variables scope in ClusterClass #6706

Cluster:

• [TBD] Add strict validation for CIDR ranges in the Cluster webhook #7538

Machine:

• [TBD] Allow user to opt-in to Node draining during Cluster delete #9692

MachineDeployment

• cleanup revisionHistoryAnnotation after ⚠️ Drop revisionHistory in MachineDeployment #12274

MachinePool:

• MachinePools cannot be upgraded atomically when used with managed providers #7248 [not planned] [TBD if blocker for MP graduation]

KubeadmConfig / KubeadmConfigTemplate / KubeadmControlPlane / KubeadmControlPlaneTemplate:

• Make KubeadmConfigTemplate immutable #4910
• kubeadm v1beta4 API:
  • Allow users to set CertificateValidityPeriod #12289
  • Bump RSA key size to 3072-bits to meet German federal BSI Technical Guideline TR-02102-2 #10077
  • Review skip Proxy and DNS in KCP and in the KubeadmConfig #12288

Finalizers:

• [TBD] Our finalizers are not domain-qualified, and therefore do not conform to requirements #10914

Backlog: (require some design/proposal)

• Cleaner separation of kubeadm and machine bootstrapping #5294
• Inconsistent provider behaviour with API server ports #5517

Done in v1.11 / v1beta2

• prepare replacing corev1.ObjectReference in v1beta2 #6539

ClusterClass & Cluster.spec.topology:

• v1beta2: Class Namespace API change #11500: ⚠️ Restructure classRef field in Cluster.spec.topology #12235
• Deprecate definitionFrom in cluster variables #10666 [deprecated] [will be removed in v1beta2]: ⚠️ Remove ClusterVariable.DefinitionFrom field #12202
• Remove IP Family from API types and cluster class builtin variables #7521 [deprecated] [will be removed in v1beta2]: ⚠️ Remove IPFamily from public APIs (move to CAPD/kind util) #12215
• Introduce InfrastructureClass (context in Add namingStrategy for InfrastructureCluster #11671 (comment)): ⚠️ Move infrastructure namingStrategy field in ClusterClass #12216
• Simplify external patch definition in ClusterClass #8210

MachineDeployment:

• Remove spec.ProgressDeadlineSeconds in MachineDeployments #11470 [deprecated] [will be removed in v1beta2]: ⚠️ Remove MD spec.progressDeadlineSeconds #12232
• Remove revision management in MachineDeployments #10479 [deprecated] [will be removed in v1beta2]: ⚠️ Drop revisionHistory in MachineDeployment #12274

KubeadmConfig / KubeadmConfigTemplate / KubeadmControlPlane / KubeadmControlPlaneTemplate:

• Implement support for kubeadm v1beta4 API #10708
• Remove experimentalRetryJoin #6942 [deprecated] [will be removed in v1beta2]: ⚠️ Remove KubeadmConfig UseExperimentalRetryJoin #12234
• Remove ClusterConfiguration.KubernetesVersion field (be careful regarding rollouts) (follow-up to: Upgrade is not updating KubeadmConfigSpec. ClusterConfiguration. KubernetesVersion  #11344)
  • Remove Networking.ServiceSubnet, Networking.PodSubnet, Domain: ⚠️ Add MaxLength & MinLength markers #11949 (comment)
  • PR: ⚠️ Remove redundant fields from CABPK / KCP ClusterConfiguration #12319

MachineHealthCheck:

• Rename conditions to nodeConditions: ⚠️ Rename MHC unhealthyConditions to unhealthyNodeConditions #12245

ClusterResourceSet:

• When the clusterresourcesetbinding is v1beta2, set the clusterName field to be required. #8267: ⚠️ Make clusterName field in ClusterResourceSetBinding required #12276

clusterctl:

• Deprecate & remove "clusterctl alpha topology plan" #10138 [deprecated]: ⚠️ Remove clusterctl alpha topology plan #12283

Done

Smaller improvements: (can be already done today)

• Add OpenAPI required tag for all required types #6397

[sbueringer]

/triage accepted

[sbueringer]

I have 3 PRs open for deprecations for v1.8:

• Deprecate definitionFrom ⚠️ Adjust CC & Cluster controller to block on variable conflicts, deprecate definitionFrom #10841
• Deprecate MHC: 🌱 Deprecate MHC MaxUnhealthy & UnhealthyRange fields #10853
• Deprecate MD revision management: 🌱 Deprecate MachineDeployment revision management #10855

[sbueringer]

Added all condition issues I could find to the list above. I think there is a huge overlap with #10897 (i.e. most of these issues would be solved if the proposal is implemented). We should check if there are gaps/conflicts