Skip to content

Commit

Permalink
fixups
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbueringer
sbueringer committed Sep 21, 2023
1 parent 12cd28d commit 8560925
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 62 deletions.
10 changes: 7 additions & 3 deletions internal/controllers/topology/cluster/cluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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} {
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions internal/controllers/topology/cluster/conditions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
83 changes: 41 additions & 42 deletions internal/controllers/topology/cluster/reconcile_state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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()
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -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()))
Expand All @@ -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()))
Expand Down Expand Up @@ -3234,6 +3232,7 @@ func newFakeMachineDeploymentTopologyState(name string, infrastructureMachineTem
}).
WithClusterName("cluster-1").
WithReplicas(1).
WithMinReadySeconds(1).
Build(),
InfrastructureMachineTemplate: infrastructureMachineTemplate.DeepCopy(),
BootstrapTemplate: bootstrapTemplate.DeepCopy(),
Expand Down
6 changes: 3 additions & 3 deletions internal/controllers/topology/cluster/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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{
Expand Down
8 changes: 8 additions & 0 deletions internal/test/builder/builders.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -1664,6 +1671,7 @@ func (m *MachineDeploymentBuilder) Build() *clusterv1.MachineDeployment {
clusterv1.ClusterNameLabel: m.clusterName,
}
}
obj.Spec.MinReadySeconds = m.minReadySeconds

return obj
}
Expand Down
19 changes: 7 additions & 12 deletions internal/test/builder/infrastructure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
},
},
},
Expand Down Expand Up @@ -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"},
Expand Down
2 changes: 2 additions & 0 deletions internal/test/envtest/environment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 8560925

Please sign in to comment.