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

⚠️ MHC: require control plane initialized, cluster infrastructure ready for node startup timeout #3752

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions api/v1alpha3/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,41 @@ package v1alpha3
import (
apiconversion "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/cluster-api/api/v1alpha4"
"sigs.k8s.io/cluster-api/util/conditions"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
)

func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*v1alpha4.Cluster)

return Convert_v1alpha3_Cluster_To_v1alpha4_Cluster(src, dst, nil)
if err := Convert_v1alpha3_Cluster_To_v1alpha4_Cluster(src, dst, nil); err != nil {
return err
}

// Given this is a bool and there is no timestamp associated with it, when this condition is set, its timestamp
// will be "now". See https://github.com/kubernetes-sigs/cluster-api/issues/3798#issuecomment-708619826 for more
// discussion.
if src.Status.ControlPlaneInitialized {
conditions.MarkTrue(dst, v1alpha4.ControlPlaneInitializedCondition)
}
vincepri marked this conversation as resolved.
Show resolved Hide resolved

return nil
}

func (dst *Cluster) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*v1alpha4.Cluster)

return Convert_v1alpha4_Cluster_To_v1alpha3_Cluster(src, dst, nil)
if err := Convert_v1alpha4_Cluster_To_v1alpha3_Cluster(src, dst, nil); err != nil {
return err
}

// Set the v1alpha3 boolean status field if the v1alpha4 condition was true
if conditions.IsTrue(src, v1alpha4.ControlPlaneInitializedCondition) {
dst.Status.ControlPlaneInitialized = true
}

return nil
}

func (src *ClusterList) ConvertTo(dstRaw conversion.Hub) error {
Expand Down Expand Up @@ -208,3 +229,7 @@ func Convert_v1alpha4_MachineRollingUpdateDeployment_To_v1alpha3_MachineRollingU
func Convert_v1alpha4_MachineHealthCheckSpec_To_v1alpha3_MachineHealthCheckSpec(in *v1alpha4.MachineHealthCheckSpec, out *MachineHealthCheckSpec, s apiconversion.Scope) error {
return autoConvert_v1alpha4_MachineHealthCheckSpec_To_v1alpha3_MachineHealthCheckSpec(in, out, s)
}

func Convert_v1alpha3_ClusterStatus_To_v1alpha4_ClusterStatus(in *ClusterStatus, out *v1alpha4.ClusterStatus, s apiconversion.Scope) error {
return autoConvert_v1alpha3_ClusterStatus_To_v1alpha4_ClusterStatus(in, out, s)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if you'd prefer I do the ControlPlaneInitialized conversion in here. It's technically more accurate, but I wanted to use the conditions.MarkTrue() helper, and that requires a full root-level object (i.e. Cluster).

}
62 changes: 57 additions & 5 deletions api/v1alpha3/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (

fuzz "github.com/google/gofuzz"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
"sigs.k8s.io/controller-runtime/pkg/conversion"

"k8s.io/apimachinery/pkg/runtime"
runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
Expand All @@ -34,11 +36,39 @@ func TestFuzzyConversion(t *testing.T) {
g.Expect(AddToScheme(scheme)).To(Succeed())
g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())

t.Run("for Cluster", utilconversion.FuzzTestFunc(scheme, &v1alpha4.Cluster{}, &Cluster{}))
t.Run("for Machine", utilconversion.FuzzTestFunc(scheme, &v1alpha4.Machine{}, &Machine{}, BootstrapFuzzFuncs))
t.Run("for MachineSet", utilconversion.FuzzTestFunc(scheme, &v1alpha4.MachineSet{}, &MachineSet{}, BootstrapFuzzFuncs))
t.Run("for MachineDeployment", utilconversion.FuzzTestFunc(scheme, &v1alpha4.MachineDeployment{}, &MachineDeployment{}, BootstrapFuzzFuncs))
t.Run("for MachineHealthCheckSpec", utilconversion.FuzzTestFunc(scheme, &v1alpha4.MachineHealthCheck{}, &MachineHealthCheck{}))
t.Run("for Cluster", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.Cluster{},
Spoke: &Cluster{},
SpokeAfterMutation: clusterSpokeAfterMutation,
}))

t.Run("for Machine", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.Machine{},
Spoke: &Machine{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
}))

t.Run("for MachineSet", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.MachineSet{},
Spoke: &MachineSet{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
}))

t.Run("for MachineDeployment", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.MachineDeployment{},
Spoke: &MachineDeployment{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
}))

t.Run("for MachineHealthCheckSpec", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.MachineHealthCheck{},
Spoke: &MachineHealthCheck{},
}))
}

func BootstrapFuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {
Expand All @@ -53,3 +83,25 @@ func BootstrapFuzzer(obj *Bootstrap, c fuzz.Continue) {
// Bootstrap.Data has been removed in v1alpha4, so setting it to nil in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
obj.Data = nil
}

// clusterSpokeAfterMutation modifies the spoke version of the Cluster such that it can pass an equality test in the
// spoke-hub-spoke conversion scenario.
func clusterSpokeAfterMutation(c conversion.Convertible) {
cluster := c.(*Cluster)

// Create a temporary 0-length slice using the same underlying array as cluster.Status.Conditions to avoid
// allocations.
tmp := cluster.Status.Conditions[:0]

for i := range cluster.Status.Conditions {
condition := cluster.Status.Conditions[i]

// Keep everything that is not ControlPlaneInitializedCondition
if condition.Type != ConditionType(v1alpha4.ControlPlaneInitializedCondition) {
tmp = append(tmp, condition)
}
}

// Point cluster.Status.Conditions and our slice that does not have ControlPlaneInitializedCondition
cluster.Status.Conditions = tmp
}
42 changes: 28 additions & 14 deletions api/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions api/v1alpha4/cluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ type ClusterStatus struct {
// +optional
InfrastructureReady bool `json:"infrastructureReady"`

// ControlPlaneInitialized defines if the control plane has been initialized.
// +optional
ControlPlaneInitialized bool `json:"controlPlaneInitialized"`

ncdc marked this conversation as resolved.
Show resolved Hide resolved
// ControlPlaneReady defines if the control plane is ready.
// +optional
ControlPlaneReady bool `json:"controlPlaneReady,omitempty"`
Expand Down
14 changes: 14 additions & 0 deletions api/v1alpha4/condition_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ const (
// Conditions and condition Reasons for the Cluster object

const (
// ControlPlaneInitializedCondition reports if the cluster's control plane has been initialized such that the
// cluster's apiserver is reachable and at least one control plane Machine has a node reference. Once this
// condition is marked true, its value is never changed. See the ControlPlaneReady condition for an indication of
// the current readiness of the cluster's control plane.
ControlPlaneInitializedCondition ConditionType = "ControlPlaneInitialized"

// MissingNodeRefReason (Severity=Info) documents a cluster waiting for at least one control plane Machine to have
// its node reference populated.
MissingNodeRefReason = "MissingNodeRef"

// WaitingForControlPlaneProviderInitializedReason (Severity=Info) documents a cluster waiting for the control plane
// provider to report successful control plane initialization.
WaitingForControlPlaneProviderInitializedReason = "WaitingForControlPlaneProviderInitialized"

// ControlPlaneReady reports the ready condition from the control plane object defined for this cluster.
// This condition is mirrored from the Ready condition in the control plane ref object, and
// the absence of this condition might signal problems in the reconcile external loops or the fact that
Expand Down
14 changes: 12 additions & 2 deletions bootstrap/kubeadm/api/v1alpha3/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

fuzz "github.com/google/gofuzz"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"

"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -34,8 +35,17 @@ func TestFuzzyConversion(t *testing.T) {
g.Expect(AddToScheme(scheme)).To(Succeed())
g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())

t.Run("for KubeadmConfig", utilconversion.FuzzTestFunc(scheme, &v1alpha4.KubeadmConfig{}, &KubeadmConfig{}, KubeadmConfigStatusFuzzFuncs))
t.Run("for KubeadmConfigTemplate", utilconversion.FuzzTestFunc(scheme, &v1alpha4.KubeadmConfigTemplate{}, &KubeadmConfigTemplate{}))
t.Run("for KubeadmConfig", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.KubeadmConfig{},
Spoke: &KubeadmConfig{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{KubeadmConfigStatusFuzzFuncs},
}))
t.Run("for KubeadmConfigTemplate", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.KubeadmConfigTemplate{},
Spoke: &KubeadmConfigTemplate{},
}))
}

func KubeadmConfigStatusFuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {
Expand Down
3 changes: 2 additions & 1 deletion bootstrap/kubeadm/controllers/kubeadmconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return ctrl.Result{}, nil
}

if !cluster.Status.ControlPlaneInitialized {
// Note: can't use IsFalse here because we need to handle the absence of the condition as well as false.
if !conditions.IsTrue(cluster, clusterv1.ControlPlaneInitializedCondition) {
return r.handleClusterNotInitialized(ctx, scope)
}

Expand Down
19 changes: 9 additions & 10 deletions bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"context"
"fmt"
"reflect"
"sigs.k8s.io/cluster-api/util/patch"
"testing"
"time"

Expand All @@ -41,6 +40,7 @@ import (
"sigs.k8s.io/cluster-api/test/helpers"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/secret"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -423,7 +423,7 @@ func TestKubeadmConfigReconciler_Reconcile_ErrorIfJoiningControlPlaneHasInvalidC
// TODO: extract this kind of code into a setup function that puts the state of objects into an initialized controlplane (implies secrets exist)
cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}
controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
controlPlaneInitConfig := newControlPlaneInitKubeadmConfig(controlPlaneInitMachine, "control-plane-init-cfg")
Expand Down Expand Up @@ -462,7 +462,7 @@ func TestKubeadmConfigReconciler_Reconcile_RequeueIfControlPlaneIsMissingAPIEndp

cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
controlPlaneInitConfig := newControlPlaneInitKubeadmConfig(controlPlaneInitMachine, "control-plane-init-cfg")

Expand Down Expand Up @@ -498,7 +498,7 @@ func TestKubeadmConfigReconciler_Reconcile_RequeueIfControlPlaneIsMissingAPIEndp
func TestReconcileIfJoinNodesAndControlPlaneIsReady(t *testing.T) {
cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}

var useCases = []struct {
Expand Down Expand Up @@ -587,7 +587,7 @@ func TestReconcileIfJoinNodePoolsAndControlPlaneIsReady(t *testing.T) {

cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}

var useCases = []struct {
Expand Down Expand Up @@ -666,7 +666,7 @@ func TestKubeadmConfigSecretCreatedStatusNotPatched(t *testing.T) {

cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}

controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
Expand Down Expand Up @@ -735,7 +735,7 @@ func TestBootstrapTokenTTLExtension(t *testing.T) {

cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}

controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
Expand Down Expand Up @@ -887,7 +887,7 @@ func TestBootstrapTokenRotationMachinePool(t *testing.T) {

cluster := newCluster("cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}

controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
Expand Down Expand Up @@ -1305,7 +1305,7 @@ func TestKubeadmConfigReconciler_Reconcile_AlwaysCheckCAVerificationUnlessReques
// Setup work for an initialized cluster
clusterName := "my-cluster"
cluster := newCluster(clusterName)
cluster.Status.ControlPlaneInitialized = true
conditions.MarkTrue(cluster, clusterv1.ControlPlaneInitializedCondition)
cluster.Status.InfrastructureReady = true
cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
Host: "example.com",
Expand Down Expand Up @@ -1433,7 +1433,6 @@ func TestKubeadmConfigReconciler_Reconcile_DoesNotFailIfCASecretsAlreadyExist(t

cluster := newCluster("my-cluster")
cluster.Status.InfrastructureReady = true
cluster.Status.ControlPlaneInitialized = false
m := newControlPlaneMachine(cluster, "control-plane-machine")
configName := "my-config"
c := newControlPlaneInitKubeadmConfig(m, configName)
Expand Down
Loading