diff --git a/internal/controllers/topology/cluster/cluster_controller_test.go b/internal/controllers/topology/cluster/cluster_controller_test.go index c05d1c746f31..1f6ce841339e 100644 --- a/internal/controllers/topology/cluster/cluster_controller_test.go +++ b/internal/controllers/topology/cluster/cluster_controller_test.go @@ -313,8 +313,10 @@ func TestClusterReconciler_reconcileUpdatesOnClusterClass(t *testing.T) { patchHelper, err := patch.NewHelper(clusterClass, env.Client) g.Expect(err).ToNot(HaveOccurred()) - // Change the infrastructureMachineTemplateName for the first of our machineDeployment and update in the API. + // Change the infrastructureMachineTemplateName for the first of our MachineDeployments and update in the API. clusterClass.Spec.Workers.MachineDeployments[0].Template.Infrastructure.Ref.Name = infrastructureMachineTemplateName2 + // Change the infrastructureMachinePoolTemplateName for the first of our MachinePools and update in the API. + clusterClass.Spec.Workers.MachinePools[0].Template.Infrastructure.Ref.Name = infrastructureMachinePoolTemplateName2 g.Expect(patchHelper.Patch(ctx, clusterClass)).To(Succeed()) g.Eventually(func(g Gomega) error { @@ -323,6 +325,7 @@ func TestClusterReconciler_reconcileUpdatesOnClusterClass(t *testing.T) { class := &clusterv1.ClusterClass{} g.Expect(env.Get(ctx, client.ObjectKey{Namespace: ns.Name, Name: actualCluster.Spec.Topology.Class}, class)).To(Succeed()) g.Expect(class.Spec.Workers.MachineDeployments[0].Template.Infrastructure.Ref.Name).To(Equal(infrastructureMachineTemplateName2)) + g.Expect(class.Spec.Workers.MachinePools[0].Template.Infrastructure.Ref.Name).To(Equal(infrastructureMachinePoolTemplateName2)) // For each cluster check that the clusterClass changes have been correctly reconciled. for _, name := range []string{clusterName1, clusterName2} { @@ -817,8 +820,8 @@ func setupTestEnvForIntegrationTests(ns *corev1.Namespace) (func() error, error) WithWorkerMachinePoolClasses(*machinePoolClass1, *machinePoolClass2, *machinePoolClass3). Build() - // 3) Two Clusters including a Cluster Topology objects and the MachineDeploymentTopology objects used in the - // Cluster Topology. The second cluster differs from the first both in version and in its MachineDeployment definition. + // 3) Two Clusters including a Cluster Topology objects and the MachineDeploymentTopology and MachinePoolTopology objects used in the + // Cluster Topology. The second cluster differs from the first both in version and in its MachineDeployment and MachinePool definitions. machineDeploymentTopology1 := builder.MachineDeploymentTopology("mdm1"). WithClass(workerClassName1 + "-md"). WithReplicas(3). @@ -860,6 +863,7 @@ func setupTestEnvForIntegrationTests(ns *corev1.Namespace) (func() error, error) Build()). Build() + // Setup kubeconfig secrets for the clusters, so the ClusterCacheTracker works. cluster1Secret := kubeconfig.GenerateSecret(cluster1, kubeconfig.FromEnvTestConfig(env.Config, cluster1)) cluster2Secret := kubeconfig.GenerateSecret(cluster2, kubeconfig.FromEnvTestConfig(env.Config, cluster2)) // Unset the ownerrefs otherwise they are invalid because they contain an empty uid. diff --git a/internal/controllers/topology/cluster/conditions_test.go b/internal/controllers/topology/cluster/conditions_test.go index 926ad8ac7184..8775ff6467cb 100644 --- a/internal/controllers/topology/cluster/conditions_test.go +++ b/internal/controllers/topology/cluster/conditions_test.go @@ -39,6 +39,7 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { g := NewWithT(t) scheme := runtime.NewScheme() g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed()) + g.Expect(expv1.AddToScheme(scheme)).To(Succeed()) deletionTime := metav1.Unix(0, 0) tests := []struct { @@ -956,8 +957,11 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { objs := []client.Object{} if tt.s != nil && tt.s.Current != nil { - for _, mds := range tt.s.Current.MachineDeployments { - objs = append(objs, mds.Object) + for _, md := range tt.s.Current.MachineDeployments { + objs = append(objs, md.Object) + } + for _, mp := range tt.s.Current.MachinePools { + objs = append(objs, mp.Object) } } for _, m := range tt.machines { diff --git a/internal/controllers/topology/cluster/reconcile_state_test.go b/internal/controllers/topology/cluster/reconcile_state_test.go index 233d5e370bf1..d9171c490d10 100644 --- a/internal/controllers/topology/cluster/reconcile_state_test.go +++ b/internal/controllers/topology/cluster/reconcile_state_test.go @@ -2212,7 +2212,7 @@ func TestReconcileMachinePools(t *testing.T) { mp2 := newFakeMachinePoolTopologyState("mp-2", infrastructureMachinePool2, bootstrapConfig2) infrastructureMachinePool2WithChanges := infrastructureMachinePool2.DeepCopy() g.Expect(unstructured.SetNestedField(infrastructureMachinePool2WithChanges.Object, "foo", "spec", "foo")).To(Succeed()) - mp2WithChangedinfrastructureMachinePool := newFakeMachinePoolTopologyState("mp-2", infrastructureMachinePool2WithChanges, bootstrapConfig2) + mp2WithChangedInfrastructureMachinePool := newFakeMachinePoolTopologyState("mp-2", infrastructureMachinePool2WithChanges, bootstrapConfig2) upgradeTrackerWithmp2PendingUpgrade := scope.NewUpgradeTracker() upgradeTrackerWithmp2PendingUpgrade.MachinePools.MarkPendingUpgrade("mp-2") @@ -2224,7 +2224,7 @@ func TestReconcileMachinePools(t *testing.T) { mp3WithChangedbootstrapConfig := newFakeMachinePoolTopologyState("mp-3", infrastructureMachinePool3, bootstrapConfig3WithChanges) bootstrapConfig3WithChangeKind := bootstrapConfig3.DeepCopy() bootstrapConfig3WithChangeKind.SetKind("AnotherGenericbootstrapConfig") - mp3WithChangedbootstrapConfigChangedKind := newFakeMachinePoolTopologyState("mp-3", infrastructureMachinePool3, bootstrapConfig3WithChanges) + mp3WithChangedbootstrapConfigChangedKind := newFakeMachinePoolTopologyState("mp-3", infrastructureMachinePool3, bootstrapConfig3WithChangeKind) infrastructureMachinePool4 := builder.TestInfrastructureMachinePool(metav1.NamespaceDefault, "infrastructure-machinepool-4").Build() bootstrapConfig4 := builder.TestBootstrapConfig(metav1.NamespaceDefault, "bootstrap-config-4").Build() @@ -2276,15 +2276,15 @@ func TestReconcileMachinePools(t *testing.T) { mp9WithInstanceSpecificTemplateMetadata.Object.Spec.Template.ObjectMeta.Labels = map[string]string{"foo": "bar"} tests := []struct { - name string - current []*scope.MachinePoolState - currentOnlyAPIServer []*scope.MachinePoolState - desired []*scope.MachinePoolState - upgradeTracker *scope.UpgradeTracker - want []*scope.MachinePoolState - wantInfrastructureMachinePoolObjectRotation map[string]bool - wantBootstrapObjectRotation map[string]bool - wantErr bool + name string + current []*scope.MachinePoolState + currentOnlyAPIServer []*scope.MachinePoolState + desired []*scope.MachinePoolState + upgradeTracker *scope.UpgradeTracker + want []*scope.MachinePoolState + wantInfrastructureMachinePoolObjectUpdate map[string]bool + wantBootstrapObjectUpdate map[string]bool + wantErr bool }{ { name: "Should create desired MachinePool if the current does not exists yet", @@ -2317,49 +2317,47 @@ func TestReconcileMachinePools(t *testing.T) { wantErr: false, }, { - name: "Should update MachinePool with infrastructureMachinePool rotation", + name: "Should update InfrastructureMachinePool", current: []*scope.MachinePoolState{mp2}, - desired: []*scope.MachinePoolState{mp2WithChangedinfrastructureMachinePool}, - want: []*scope.MachinePoolState{mp2WithChangedinfrastructureMachinePool}, - wantInfrastructureMachinePoolObjectRotation: map[string]bool{"mp-2": true}, + desired: []*scope.MachinePoolState{mp2WithChangedInfrastructureMachinePool}, + want: []*scope.MachinePoolState{mp2WithChangedInfrastructureMachinePool}, + wantInfrastructureMachinePoolObjectUpdate: map[string]bool{"mp-2": true}, wantErr: false, }, { - name: "Should not update MachinePool if MachinePool is pending upgrade", + name: "Should not update InfrastructureMachinePool if MachinePool is pending upgrade", current: []*scope.MachinePoolState{mp2}, - desired: []*scope.MachinePoolState{mp2WithChangedinfrastructureMachinePool}, + desired: []*scope.MachinePoolState{mp2WithChangedInfrastructureMachinePool}, upgradeTracker: upgradeTrackerWithmp2PendingUpgrade, want: []*scope.MachinePoolState{mp2}, - wantInfrastructureMachinePoolObjectRotation: map[string]bool{"mp-2": false}, + wantInfrastructureMachinePoolObjectUpdate: map[string]bool{"mp-2": false}, wantErr: false, }, { - name: "Should update MachinePool with bootstrapConfig rotation", - current: []*scope.MachinePoolState{mp3}, - desired: []*scope.MachinePoolState{mp3WithChangedbootstrapConfig}, - want: []*scope.MachinePoolState{mp3WithChangedbootstrapConfig}, - wantBootstrapObjectRotation: map[string]bool{"mp-3": true}, - wantErr: false, + name: "Should update BootstrapConfig", + current: []*scope.MachinePoolState{mp3}, + desired: []*scope.MachinePoolState{mp3WithChangedbootstrapConfig}, + want: []*scope.MachinePoolState{mp3WithChangedbootstrapConfig}, + wantBootstrapObjectUpdate: map[string]bool{"mp-3": true}, + wantErr: false, }, { - name: "Should update MachinePool with bootstrapConfig rotation with changed kind", - current: []*scope.MachinePoolState{mp3}, - desired: []*scope.MachinePoolState{mp3WithChangedbootstrapConfigChangedKind}, - want: []*scope.MachinePoolState{mp3WithChangedbootstrapConfigChangedKind}, - wantBootstrapObjectRotation: map[string]bool{"mp-3": true}, - wantErr: false, + name: "Should fail update MachinePool because of changed BootstrapConfig kind", + current: []*scope.MachinePoolState{mp3}, + desired: []*scope.MachinePoolState{mp3WithChangedbootstrapConfigChangedKind}, + wantErr: true, }, { - name: "Should update MachinePool with infrastructureMachinePool and bootstrapConfig rotation", + name: "Should update InfrastructureMachinePool and BootstrapConfig", current: []*scope.MachinePoolState{mp4}, desired: []*scope.MachinePoolState{mp4WithChangedObjects}, want: []*scope.MachinePoolState{mp4WithChangedObjects}, - wantInfrastructureMachinePoolObjectRotation: map[string]bool{"mp-4": true}, - wantBootstrapObjectRotation: map[string]bool{"mp-4": true}, - wantErr: false, + wantInfrastructureMachinePoolObjectUpdate: map[string]bool{"mp-4": true}, + wantBootstrapObjectUpdate: map[string]bool{"mp-4": true}, + wantErr: false, }, { - name: "Should fail update MachinePool because of changed infrastructureMachinePool kind", + name: "Should fail update MachinePool because of changed InfrastructureMachinePool kind", current: []*scope.MachinePoolState{mp5}, desired: []*scope.MachinePoolState{mp5WithChangedinfrastructureMachinePoolKind}, wantErr: true, @@ -2382,9 +2380,9 @@ func TestReconcileMachinePools(t *testing.T) { current: []*scope.MachinePoolState{mp8Update, mp8Delete}, desired: []*scope.MachinePoolState{mp8Create, mp8UpdateWithChangedObjects}, want: []*scope.MachinePoolState{mp8Create, mp8UpdateWithChangedObjects}, - wantInfrastructureMachinePoolObjectRotation: map[string]bool{"mp-8-update": true}, - wantBootstrapObjectRotation: map[string]bool{"mp-8-update": true}, - wantErr: false, + wantInfrastructureMachinePoolObjectUpdate: map[string]bool{"mp-8-update": true}, + wantBootstrapObjectUpdate: map[string]bool{"mp-8-update": true}, + wantErr: false, }, { name: "Enforce template metadata", @@ -2493,9 +2491,9 @@ func TestReconcileMachinePools(t *testing.T) { g.Expect(&gotBootstrapObject).To(EqualObject(wantMachinePoolState.BootstrapObject, IgnoreAutogeneratedMetadata, IgnoreNameGenerated)) - // Check BootstrapObject rotation if there was a previous MachinePool. + // Check BootstrapObject update. if currentMachinePoolState != nil && currentMachinePoolState.BootstrapObject != nil { - if tt.wantBootstrapObjectRotation[gotMachinePool.Name] { + if tt.wantBootstrapObjectUpdate[gotMachinePool.Name] { g.Expect(currentMachinePoolState.BootstrapObject.GetResourceVersion()).ToNot(Equal(gotBootstrapObject.GetResourceVersion())) } else { g.Expect(currentMachinePoolState.BootstrapObject.GetResourceVersion()).To(Equal(gotBootstrapObject.GetResourceVersion())) @@ -2517,9 +2515,9 @@ func TestReconcileMachinePools(t *testing.T) { g.Expect(&gotInfrastructureMachinePoolObject).To(EqualObject(wantMachinePoolState.InfrastructureMachinePoolObject, IgnoreAutogeneratedMetadata, IgnoreNameGenerated)) - // Check InfrastructureMachinePoolObject rotation if there was a previous MachinePool. + // Check InfrastructureMachinePoolObject update. if currentMachinePoolState != nil && currentMachinePoolState.InfrastructureMachinePoolObject != nil { - if tt.wantInfrastructureMachinePoolObjectRotation[gotMachinePool.Name] { + if tt.wantInfrastructureMachinePoolObjectUpdate[gotMachinePool.Name] { g.Expect(currentMachinePoolState.InfrastructureMachinePoolObject.GetResourceVersion()).ToNot(Equal(gotInfrastructureMachinePoolObject.GetResourceVersion())) } else { g.Expect(currentMachinePoolState.InfrastructureMachinePoolObject.GetResourceVersion()).To(Equal(gotInfrastructureMachinePoolObject.GetResourceVersion())) @@ -3234,6 +3232,7 @@ func newFakeMachineDeploymentTopologyState(name string, infrastructureMachineTem }). WithClusterName("cluster-1"). WithReplicas(1). + WithMinReadySeconds(1). Build(), InfrastructureMachineTemplate: infrastructureMachineTemplate.DeepCopy(), BootstrapTemplate: bootstrapTemplate.DeepCopy(), diff --git a/internal/controllers/topology/cluster/suite_test.go b/internal/controllers/topology/cluster/suite_test.go index 2ec84f338c1f..2c595a67dc87 100644 --- a/internal/controllers/topology/cluster/suite_test.go +++ b/internal/controllers/topology/cluster/suite_test.go @@ -65,9 +65,6 @@ func TestMain(m *testing.M) { } } setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { - // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers - // requiring a connection to a remote cluster - log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") unstructuredCachingClient, err := client.New(mgr.GetConfig(), client.Options{ Cache: &client.CacheOptions{ Reader: mgr.GetCache(), @@ -77,6 +74,9 @@ func TestMain(m *testing.M) { if err != nil { panic(fmt.Sprintf("unable to create unstructuredCachineClient: %v", err)) } + // Set up a ClusterCacheTracker and ClusterCacheReconciler to provide to controllers + // requiring a connection to a remote cluster + log := ctrl.Log.WithName("remote").WithName("ClusterCacheTracker") secretCachingClient, err := client.New(mgr.GetConfig(), client.Options{ HTTPClient: mgr.GetHTTPClient(), Cache: &client.CacheOptions{ diff --git a/internal/test/builder/builders.go b/internal/test/builder/builders.go index 6bc97efcc058..285831a69f81 100644 --- a/internal/test/builder/builders.go +++ b/internal/test/builder/builders.go @@ -1557,6 +1557,7 @@ type MachineDeploymentBuilder struct { generation *int64 labels map[string]string status *clusterv1.MachineDeploymentStatus + minReadySeconds *int32 } // MachineDeployment creates a MachineDeploymentBuilder with the given name and namespace. @@ -1621,6 +1622,12 @@ func (m *MachineDeploymentBuilder) WithStatus(status clusterv1.MachineDeployment return m } +// WithMinReadySeconds sets the passed value on the machine deployment spec. +func (m *MachineDeploymentBuilder) WithMinReadySeconds(minReadySeconds int32) *MachineDeploymentBuilder { + m.minReadySeconds = &minReadySeconds + return m +} + // Build creates a new MachineDeployment with the variables and objects passed to the MachineDeploymentBuilder. func (m *MachineDeploymentBuilder) Build() *clusterv1.MachineDeployment { obj := &clusterv1.MachineDeployment{ @@ -1664,6 +1671,7 @@ func (m *MachineDeploymentBuilder) Build() *clusterv1.MachineDeployment { clusterv1.ClusterNameLabel: m.clusterName, } } + obj.Spec.MinReadySeconds = m.minReadySeconds return obj } diff --git a/internal/test/builder/infrastructure.go b/internal/test/builder/infrastructure.go index 301def9d688b..dfcb91775061 100644 --- a/internal/test/builder/infrastructure.go +++ b/internal/test/builder/infrastructure.go @@ -37,9 +37,13 @@ var ( // GenericInfrastructureMachinePoolTemplateKind is the Kind for the GenericInfrastructureMachinePoolTemplate. GenericInfrastructureMachinePoolTemplateKind = "GenericInfrastructureMachinePoolTemplate" + // GenericInfrastructureMachinePoolTemplateCRD is a generic infrastructure machine pool template CRD. + GenericInfrastructureMachinePoolTemplateCRD = untypedCRD(InfrastructureGroupVersion.WithKind(GenericInfrastructureMachinePoolTemplateKind)) // GenericInfrastructureMachinePoolKind is the Kind for the GenericInfrastructureMachinePool. GenericInfrastructureMachinePoolKind = "GenericInfrastructureMachinePool" + // GenericInfrastructureMachinePoolCRD is a generic infrastructure machine pool CRD. + GenericInfrastructureMachinePoolCRD = untypedCRD(InfrastructureGroupVersion.WithKind(GenericInfrastructureMachinePoolKind)) // GenericInfrastructureClusterKind is the kind for the GenericInfrastructureCluster type. GenericInfrastructureClusterKind = "GenericInfrastructureCluster" @@ -70,7 +74,7 @@ var ( // TestInfrastructureMachinePoolTemplateKind is the kind for the TestInfrastructureMachinePoolTemplate type. TestInfrastructureMachinePoolTemplateKind = "TestInfrastructureMachinePoolTemplate" - // TestInfrastructureMachinePoolTemplateCRD is a test infrastructure machine template CRD. + // TestInfrastructureMachinePoolTemplateCRD is a test infrastructure machine pool template CRD. TestInfrastructureMachinePoolTemplateCRD = testInfrastructureMachinePoolTemplateCRD(InfrastructureGroupVersion.WithKind(TestInfrastructureMachinePoolTemplateKind)) // TestInfrastructureMachinePoolKind is the kind for the TestInfrastructureMachinePool type. @@ -165,9 +169,8 @@ func testInfrastructureMachinePoolTemplateCRD(gvk schema.GroupVersionKind) *apie "template": { Type: "object", Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "clusterName": {Type: "string"}, - "metadata": metadataSchema, - "spec": machineSpecSchema, + "metadata": metadataSchema, + "spec": machinePoolSpecSchema, }, }, }, @@ -277,14 +280,6 @@ var ( }, }, }, - "template": { - Type: "object", - Properties: map[string]apiextensionsv1.JSONSchemaProps{ - "metadata": metadataSchema, - "spec": machineSpecSchema, - }, - }, - "clusterName": {Type: "string"}, // General purpose fields to be used in different test scenario. "foo": {Type: "string"}, "bar": {Type: "string"}, diff --git a/internal/test/envtest/environment.go b/internal/test/envtest/environment.go index 7ac0259a963b..1d8668d1d59e 100644 --- a/internal/test/envtest/environment.go +++ b/internal/test/envtest/environment.go @@ -212,6 +212,8 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment { builder.GenericControlPlaneTemplateCRD.DeepCopy(), builder.GenericInfrastructureMachineCRD.DeepCopy(), builder.GenericInfrastructureMachineTemplateCRD.DeepCopy(), + builder.GenericInfrastructureMachinePoolCRD.DeepCopy(), + builder.GenericInfrastructureMachinePoolTemplateCRD.DeepCopy(), builder.GenericInfrastructureClusterCRD.DeepCopy(), builder.GenericInfrastructureClusterTemplateCRD.DeepCopy(), builder.GenericRemediationCRD.DeepCopy(),