Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabriziopandini
fabriziopandini committed Oct 15, 2024
1 parent 34202b3 commit 5792866
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 48 deletions.
2 changes: 1 addition & 1 deletion docs/book/src/developer/core/controllers/cluster.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ an ControlPlane object, e.g. KubeadmControlPlane, EKSControlPlane etc.

Among those rules:
- ControlPlane SHOULD report a [controlplane endpoint](../../providers/contracts/control-plane.md#controlplane-endpoint) for the Cluster
- ControlPlane MUST report when Cluster's infrastructure is [fully provisioned](../../providers/contracts/control-plane.md#controlplane-initialization-completed)
- ControlPlane MUST report when Cluster's control plane is [fully provisioned](../../providers/contracts/control-plane.md#controlplane-initialization-completed)
- ControlPlane MUST manage a [KubeConfig secret](../../providers/contracts/control-plane.md#cluster-kubeconfig-management)
- ControlPlane SHOULD report [conditions](../../providers/contracts/control-plane.md#controlplane-conditions)
- ControlPlane SHOULD report [terminal failures](../../providers/contracts/control-plane.md#controlplane-terminal-failures)
Expand Down
6 changes: 0 additions & 6 deletions docs/book/src/developer/providers/contracts/bootstrap-config.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,6 @@ the implication of this choice which are described both in the document above an

</aside>

### BootstrapConfig: pausing

Providers SHOULD implement the pause behaviour for every object with a reconciliation loop. This is done by checking if `spec.paused` is set on the Cluster object and by checking for the `cluster.x-k8s.io/paused` annotation on the BootstrapConfig object.

If implementing the pause behavior, providers SHOULD surface the paused status of an object using the Paused condition: `Status.Conditions[Paused]`.

### BootstrapConfig: terminal failures

Each BootstrapConfig SHOULD report when BootstrapConfig's enter in a state that cannot be recovered (terminal failure) by
Expand Down
80 changes: 45 additions & 35 deletions docs/book/src/developer/providers/contracts/control-plane.md
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Contract rules for ControlPlane

Control plane providers MUST implement an ControlPlane resource.
Control plane providers MUST implement a ControlPlane resource.

The goal of an ControlPlane resource is to instantiate a Kubernetes control plane; a Kubernetes control plane
The goal of a ControlPlane resource is to instantiate a Kubernetes control plane; a Kubernetes control plane
at least contains the following components:
- Kubernetes API Server
- Kubernetes Controller Manager
Expand All @@ -14,7 +14,7 @@ Optional control plane components are
- Cluster DNS (e.g. CoreDNS)
- Service proxy (e.g. kube-proxy)

Instead, CNI should be left to user to apply once control plane is instantiated.
Instead, CNI should be left to users to apply once the control plane is instantiated.

The ControlPlane resource will be referenced by one of the Cluster API core resources, Cluster.

Expand Down Expand Up @@ -43,7 +43,7 @@ When developing a provider, you MUST consider any Cluster API behaviour that is
as a Cluster API internal implementation detail, and internal implementation details can change at any time.

Accordingly, in order to not expose users to the risk that your provider breaks when the Cluster API internal behavior
changes, you MUST NOT rely on any Cluster API internal behaviour when implementing an ControlPlane resource.
changes, you MUST NOT rely on any Cluster API internal behaviour when implementing a ControlPlane resource.

Instead, whenever you need something more from the Cluster API contract, you MUST engage the community.

Expand All @@ -57,28 +57,28 @@ repo or add an item to the agenda in the [Cluster API community meeting](https:/

## Rules (contract version v1beta1)

| Rule | Mandatory | Note |
|----------------------------------------------------------------------|-----------|------------------------------------------------------------------------------------------------------------------------------------|
| [All resources: scope] | Yes | |
| [All resources: `TypeMeta` and `ObjectMeta`field] | Yes | |
| [All resources: `APIVersion` field value] | Yes | |
| [ControlPlane, ControlPlaneList resource definition] | Yes | |
| [ControlPlane: endpoint] | No | Mandatory if control plane endpoint is not provided by other means. |
| [ControlPlane: replicas] | No | Mandatory if control plane as a notion of number of instances; also mandatory for cluster class support. |
| [ControlPlane: version] | No | Mandatory if control plane allows direct management of the Kubernetes version in use; Mandatory for cluster class support. |
| [ControlPlane: machines] | No | Mandatory if control plane instances are represented with a set of Cluster API Machines; also mandatory for cluster class support. |
| [ControlPlane: initialization completed] | Yes | |
| [ControlPlane: conditions] | No | |
| [ControlPlane: terminal failures] | No | |
| [ControlPlaneTemplate, ControlPlaneTemplateList resource definition] | No | Mandatory for ClusterClasses support |
| [Cluster kubeconfig management] | Yes | |
| [Cluster certificate management] | No | |
| [Machine placement] | No | |
| [Metadata propagation] | No | |
| [MinReadySeconds and UpToDate propagation] | No | |
| [Support for running multiple instances] | No | Mandatory for clusterctl CLI support |
| [Clusterctl support] | No | Mandatory for clusterctl CLI support |
| [ControlPlane: pausing] | No | |
| Rule | Mandatory | Note |
|----------------------------------------------------------------------|-----------|----------------------------------------------------------------------------------------------------------------------------|
| [All resources: scope] | Yes | |
| [All resources: `TypeMeta` and `ObjectMeta`field] | Yes | |
| [All resources: `APIVersion` field value] | Yes | |
| [ControlPlane, ControlPlaneList resource definition] | Yes | |
| [ControlPlane: endpoint] | No | Mandatory if control plane endpoint is not provided by other means. |
| [ControlPlane: replicas] | No | Mandatory if control plane has a notion of number of instances. |
| [ControlPlane: version] | No | Mandatory if control plane allows direct management of the Kubernetes version in use; Mandatory for cluster class support. |
| [ControlPlane: machines] | No | Mandatory if control plane instances are represented with a set of Cluster API Machines. |
| [ControlPlane: initialization completed] | Yes | |
| [ControlPlane: conditions] | No | |
| [ControlPlane: terminal failures] | No | |
| [ControlPlaneTemplate, ControlPlaneTemplateList resource definition] | No | Mandatory for ClusterClasses support |
| [Cluster kubeconfig management] | Yes | |
| [Cluster certificate management] | No | |
| [Machine placement] | No | |
| [Metadata propagation] | No | |
| [MinReadySeconds and UpToDate propagation] | No | |
| [Support for running multiple instances] | No | Mandatory for clusterctl CLI support |
| [Clusterctl support] | No | Mandatory for clusterctl CLI support |
| [ControlPlane: pausing] | No | |

### All resources: scope

Expand Down Expand Up @@ -116,7 +116,6 @@ rules:
- controlplane.foo.com
resources:
- foocontrolplanes
- foocontrolplanetemplates
verbs:
- create
- delete
Expand All @@ -125,10 +124,21 @@ rules:
- patch
- update
- watch
- apiGroups:
- infrastructure.foo.com
resources:
- foocontrolplanetemplates
verbs:
- get
- list
- patch
- update
- watch
```
Note: The write permissions allow the Cluster controller to set owner references and labels on the ControlPlane resources;
when using ClusterClass and managed topologies, also ControlPlaneTemplates are managed directly by Cluster API.
write permissions are not used for general mutations of ControlPlane resources, unless specifically required (e.g. when
using ClusterClass and managed topologies).
#### All resources: version
Expand Down Expand Up @@ -507,7 +517,7 @@ type FooControlPlaneStatus struct {
Once `status.initialized` and `status.ready` are set, the Cluster "core" controller will bubbles up those info in
Cluster's `status.controlPlaneReady` field and in the `ControlPlaneInitialized` condition.

If defined, also InfraCluster's `spec.controlPlaneEndpoint` will be surfaced on Cluster's corresponding fields at the same time.
If defined, also ControlPlane's `spec.controlPlaneEndpoint` will be surfaced on Cluster's corresponding fields at the same time.

<aside class="note warning">

Expand Down Expand Up @@ -560,7 +570,7 @@ Additional considerations apply specifically to the ControlPlane resource:
In order to disambiguate the usage of the ready term and improve how the status of the control plane is
presented, Cluster API will stop surfacing the `Ready` condition and instead surface a new `Available` condition.

The the `Available` condition is expected to properly represents the fact that a ControlPlane can be operational
The `Available` condition is expected to properly represents the fact that a ControlPlane can be operational
even if there is a certain degree of not readiness / disruption in the system, or if lifecycle operations are happening.

Last, but not least, in order to ensure a consistent users experience, it is also recommended to consider aligning also other
Expand All @@ -583,15 +593,15 @@ setting `status.failureReason` and `status.failureMessage` in the ControlPlane r

```go
type FoControlPlaneStatus struct {
// FailureReason will be set in the event that there is a terminal problem reconciling the FooControlPlane
// failureReason will be set in the event that there is a terminal problem reconciling the FooControlPlane
// and will contain a succinct value suitable for machine interpretation.
//
// This field should not be set for transitive errors that can be fixed automatically or with manual intervention,
// but instead indicate that something is fundamentally wrong with the FooCluster and that it cannot be recovered.
// +optional
FailureReason *capierrors.ClusterStatusError `json:"failureReason,omitempty"`

// FailureMessage will be set in the event that there is a terminal problem reconciling the FooControlPlane
// failureMessage will be set in the event that there is a terminal problem reconciling the FooControlPlane
// and will contain a more verbose string suitable for logging and human consumption.
//
// This field should not be set for transitive errors that can be fixed automatically or with manual intervention,
Expand Down Expand Up @@ -679,11 +689,11 @@ with other users or applications build on top of Cluster API. Instead, follow in
to create custom certificates for additional users or other applications.

The kubeconfig secret MUST:
- Be crated in the same namespace where the Cluster exists
- Be created in the same namespace where the Cluster exists
- Be named `<cluster>-kubeconfig`
- Have type `cluster.x-k8s.io/secret`
- Be labelled with the key-pair `cluster.x-k8s.io/cluster-name=${CLUSTER_NAME}`.
Note: this label is required for the secret to retrievable in the cache used by CAPI managers.
Note: this label is required for the secret to be retrievable in the cache used by CAPI managers.

Important! If a control plane provider uses client certificates for authentication in these Kubeconfigs, the client certificate
MUST be kept with a reasonably short expiration period and periodically regenerated to keep a valid set of credentials available.
Expand All @@ -698,7 +708,7 @@ Cluster certificates MUST be stored as a secrets:
- Following a naming convention `<cluster>-<certificate>`; common certificate names are `ca`, `etcd`, `proxy`, `sa`
- Have type `cluster.x-k8s.io/secret`
- Be labelled with the key-pair `cluster.x-k8s.io/cluster-name=${CLUSTER_NAME}`.
Note: this label is required for the secret to retrievable in the cache used by CAPI managers.
Note: this label is required for the secret to be retrievable in the cache used by CAPI managers.

See [Certificate Management] for more context.

Expand Down
12 changes: 6 additions & 6 deletions docs/book/src/developer/providers/contracts/infra-machine.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,12 +397,6 @@ See [Improving status in CAPI resources].

</aside>

### InfraMachine: pausing

Providers SHOULD implement the pause behaviour for every object with a reconciliation loop. This is done by checking if `spec.paused` is set on the Cluster object and by checking for the `cluster.x-k8s.io/paused` annotation on the InfraMachine object.

If implementing the pause behavior, providers SHOULD surface the paused status of an object using the Paused condition: `Status.Conditions[Paused]`.

### InfraMachineTemplate, InfraMachineTemplateList resource definition

For a given InfraMachine resource, you MUST also add a corresponding InfraMachineTemplate resources in order to use it
Expand Down Expand Up @@ -492,6 +486,12 @@ Please, read carefully the page linked above to fully understand implications an

The clusterctl command is designed to work with all the providers compliant with the rules defined in the [clusterctl provider contract].

### InfraMachine: pausing

Providers SHOULD implement the pause behaviour for every object with a reconciliation loop. This is done by checking if `spec.paused` is set on the Cluster object and by checking for the `cluster.x-k8s.io/paused` annotation on the InfraMachine object.

If implementing the pause behavior, providers SHOULD surface the paused status of an object using the Paused condition: `Status.Conditions[Paused]`.

## Typical InfraMachine reconciliation workflow

A machine infrastructure provider must respond to changes to its InfraMachine resources. This process is
Expand Down

0 comments on commit 5792866

Please sign in to comment.