Skip to content

Commit

Permalink
test/framework: WaitForControlPlaneToBeReady now considers replicas
Browse files Browse the repository at this point in the history 
Signed-off-by: Stefan Büringer [email protected]
  • Loading branch information
@sbueringer
sbueringer committed Jan 13, 2023
1 parent e499366 commit 6c5eb9c
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 68 deletions.
3 changes: 2 additions & 1 deletion internal/contract/controlplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ func (c *ControlPlaneContract) IsUpgrading(obj *unstructured.Unstructured) (bool
// - spec.replicas != status.replicas.
// - spec.replicas != status.updatedReplicas.
// - spec.replicas != status.readyReplicas.
// - status.unavailableReplicas > 0.
func (c *ControlPlaneContract) IsScaling(obj *unstructured.Unstructured) (bool, error) {
desiredReplicas, err := c.Replicas().Get(obj)
if err != nil {
Expand Down Expand Up @@ -247,7 +248,7 @@ func (c *ControlPlaneContract) IsScaling(obj *unstructured.Unstructured) (bool,
}

unavailableReplicas, err := c.UnavailableReplicas().Get(obj)
if err != nil && !errors.Is(err, errNotFound) {
if err != nil {
if !errors.Is(err, errNotFound) {
return false, errors.Wrap(err, "failed to get control plane status unavailableReplicas")
}
Expand Down
92 changes: 51 additions & 41 deletions internal/controllers/topology/cluster/desired_state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ func TestComputeControlPlaneVersion(t *testing.T) {
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.RuntimeSDK, true)()

// Note: the version used by the machine deployments does
// not affect how we determining the control plane version.
// not affect how we're determining the control plane version.
// We only want to know if the machine deployments are stable.
//
// A machine deployment is considered stable if all the following are true:
Expand Down Expand Up @@ -706,10 +706,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build(),
expectedVersion: "v1.2.3",
Expand Down Expand Up @@ -739,10 +740,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(1),
"status.updatedReplicas": int64(1),
"status.readyReplicas": int64(1),
"status.version": "v1.2.2",
"status.replicas": int64(1),
"status.updatedReplicas": int64(1),
"status.readyReplicas": int64(1),
"status.unavailableReplicas": int64(0),
}).
Build(),
expectedVersion: "v1.2.2",
Expand All @@ -756,10 +758,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build(),
machineDeploymentsState: scope.MachineDeploymentsStateMap{
Expand All @@ -778,10 +781,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build(),
machineDeploymentsState: scope.MachineDeploymentsStateMap{
Expand All @@ -800,10 +804,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build(),
machineDeploymentsState: scope.MachineDeploymentsStateMap{
Expand All @@ -822,10 +827,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build(),
machineDeploymentsState: scope.MachineDeploymentsStateMap{
Expand Down Expand Up @@ -1211,10 +1217,11 @@ func TestComputeControlPlaneVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build()

Expand Down Expand Up @@ -1697,10 +1704,11 @@ func TestComputeMachineDeploymentVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.2",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build()
controlPlaneStable123 := builder.ControlPlane("test1", "cp1").
Expand All @@ -1709,10 +1717,11 @@ func TestComputeMachineDeploymentVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.3",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": "v1.2.3",
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build()
controlPlaneUpgrading := builder.ControlPlane("test1", "cp1").
Expand All @@ -1729,10 +1738,11 @@ func TestComputeMachineDeploymentVersion(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": "v1.2.3",
"status.replicas": int64(1),
"status.updatedReplicas": int64(1),
"status.readyReplicas": int64(1),
"status.version": "v1.2.3",
"status.replicas": int64(1),
"status.updatedReplicas": int64(1),
"status.readyReplicas": int64(1),
"status.unavailableReplicas": int64(0),
}).
Build()
controlPlaneDesired := builder.ControlPlane("test1", "cp1").
Expand Down
36 changes: 20 additions & 16 deletions internal/controllers/topology/cluster/reconcile_state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,11 @@ func TestReconcile_callAfterClusterUpgrade(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": topologyVersion,
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": topologyVersion,
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build()
controlPlaneStableAtLowerVersion := builder.ControlPlane("test1", "cp1").
Expand All @@ -510,10 +511,11 @@ func TestReconcile_callAfterClusterUpgrade(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": lowerVersion,
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": lowerVersion,
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build()
controlPlaneUpgrading := builder.ControlPlane("test1", "cp1").
Expand All @@ -522,10 +524,11 @@ func TestReconcile_callAfterClusterUpgrade(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": lowerVersion,
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.version": lowerVersion,
"status.replicas": int64(2),
"status.updatedReplicas": int64(2),
"status.readyReplicas": int64(2),
"status.unavailableReplicas": int64(0),
}).
Build()
controlPlaneScaling := builder.ControlPlane("test1", "cp1").
Expand All @@ -534,10 +537,11 @@ func TestReconcile_callAfterClusterUpgrade(t *testing.T) {
"spec.replicas": int64(2),
}).
WithStatusFields(map[string]interface{}{
"status.version": topologyVersion,
"status.replicas": int64(1),
"status.updatedReplicas": int64(1),
"status.readyReplicas": int64(1),
"status.version": topologyVersion,
"status.replicas": int64(1),
"status.updatedReplicas": int64(1),
"status.readyReplicas": int64(1),
"status.unavailableReplicas": int64(0),
}).
Build()

Expand Down
1 change: 1 addition & 0 deletions test/e2e/clusterctl_upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ func ClusterctlUpgradeSpec(ctx context.Context, inputGetter func() ClusterctlUpg
}

// After the upgrade check that there were no unexpected rollouts.
log.Logf("Verify there are no unexpected rollouts")
Consistently(func() bool {
postUpgradeMachineList := &unstructured.UnstructuredList{}
postUpgradeMachineList.SetGroupVersionKind(clusterv1.GroupVersion.WithKind("MachineList"))
Expand Down
9 changes: 8 additions & 1 deletion test/e2e/self_hosted.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ func SelfHostedSpec(ctx context.Context, inputGetter func() SelfHostedSpecInput)
Expect(controlPlane).ToNot(BeNil())

// After the move check that there were no unexpected rollouts.
log.Logf("Verify there are no unexpected rollouts")
Consistently(func() bool {
postMoveMachineList := &unstructured.UnstructuredList{}
postMoveMachineList.SetGroupVersionKind(clusterv1.GroupVersion.WithKind("MachineList"))
Expand All @@ -279,8 +280,14 @@ func SelfHostedSpec(ctx context.Context, inputGetter func() SelfHostedSpecInput)
return
}

By("Upgrading the self-hosted Cluster")
log.Logf("Waiting for control plane to be ready")
framework.WaitForControlPlaneAndMachinesReady(ctx, framework.WaitForControlPlaneAndMachinesReadyInput{
GetLister: selfHostedClusterProxy.GetClient(),
Cluster: clusterResources.Cluster,
ControlPlane: clusterResources.ControlPlane,
}, input.E2EConfig.GetIntervals(specName, "wait-control-plane")...)

By("Upgrading the self-hosted Cluster")
if clusterResources.Cluster.Spec.Topology != nil {
// Cluster is using ClusterClass, upgrade via topology.
By("Upgrading the Cluster topology")
Expand Down
31 changes: 22 additions & 9 deletions test/framework/controlplane_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -164,20 +163,34 @@ type WaitForControlPlaneToBeReadyInput struct {
func WaitForControlPlaneToBeReady(ctx context.Context, input WaitForControlPlaneToBeReadyInput, intervals ...interface{}) {
By("Waiting for the control plane to be ready")
controlplane := &controlplanev1.KubeadmControlPlane{}
Eventually(func() (controlplanev1.KubeadmControlPlane, error) {
Eventually(func() (bool, error) {
key := client.ObjectKey{
Namespace: input.ControlPlane.GetNamespace(),
Name: input.ControlPlane.GetName(),
}
if err := input.Getter.Get(ctx, key, controlplane); err != nil {
return *controlplane, errors.Wrapf(err, "failed to get KCP")
return false, errors.Wrapf(err, "failed to get KCP")
}

desiredReplicas := controlplane.Spec.Replicas
statusReplicas := controlplane.Status.Replicas
updatedReplicas := controlplane.Status.UpdatedReplicas
readyReplicas := controlplane.Status.ReadyReplicas
unavailableReplicas := controlplane.Status.UnavailableReplicas

// Control plane is still rolling out (and thus not ready) if:
// * .spec.replicas, .status.replicas, .status.updatedReplicas,
// .status.readyReplicas are not equal and
// * unavailableReplicas > 0
if statusReplicas != *desiredReplicas ||
updatedReplicas != *desiredReplicas ||
readyReplicas != *desiredReplicas ||
unavailableReplicas > 0 {
return false, nil
}
return *controlplane, nil
}, intervals...).Should(MatchFields(IgnoreExtras, Fields{
"Status": MatchFields(IgnoreExtras, Fields{
"Ready": BeTrue(),
}),
}), PrettyPrint(controlplane)+"\n")

return true, nil
}, intervals...).Should(BeTrue(), PrettyPrint(controlplane)+"\n")
}

// AssertControlPlaneFailureDomainsInput is the input for AssertControlPlaneFailureDomains.
Expand Down

0 comments on commit 6c5eb9c

Please sign in to comment.