From 94792d85dea0865a7d4658a9f28408f543e0a088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20K=C5=99epinsk=C3=BD?= Date: Wed, 15 Feb 2023 15:05:50 +0100 Subject: [PATCH] simplify TestValidateControllersOptions test --- .../app/options/options_test.go | 166 +++++++++--------- 1 file changed, 84 insertions(+), 82 deletions(-) diff --git a/cmd/kube-controller-manager/app/options/options_test.go b/cmd/kube-controller-manager/app/options/options_test.go index 335e97b600c..571a64dbea7 100644 --- a/cmd/kube-controller-manager/app/options/options_test.go +++ b/cmd/kube-controller-manager/app/options/options_test.go @@ -660,24 +660,26 @@ func TestValidateControllersOptions(t *testing.T) { name string expectErrors bool expectedErrorSubString string - validate func() []error + options interface { + Validate() []error + } }{ { name: "AttachDetachControllerOptions reconciler sync loop period less than one second", expectErrors: true, expectedErrorSubString: "duration time must be greater than one second", - validate: (&AttachDetachControllerOptions{ + options: &AttachDetachControllerOptions{ &attachdetachconfig.AttachDetachControllerConfiguration{ ReconcilerSyncLoopPeriod: metav1.Duration{Duration: time.Second / 2}, DisableAttachDetachReconcilerSync: true, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions KubeletServingSignerConfiguration no cert file", expectErrors: true, expectedErrorSubString: "cannot specify key without cert", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -699,13 +701,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions KubeletServingSignerConfiguration no key file", expectErrors: true, expectedErrorSubString: "cannot specify cert without key", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -727,13 +729,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions KubeletClientSignerConfiguration no cert file", expectErrors: true, expectedErrorSubString: "cannot specify key without cert", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -755,13 +757,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions KubeletClientSignerConfiguration no key file", expectErrors: true, expectedErrorSubString: "cannot specify cert without key", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -783,13 +785,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions KubeAPIServerClientSignerConfiguration no cert file", expectErrors: true, expectedErrorSubString: "cannot specify key without cert", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -811,13 +813,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions KubeAPIServerClientSignerConfiguration no key file", expectErrors: true, expectedErrorSubString: "cannot specify cert without key", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -839,13 +841,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions LegacyUnknownSignerConfiguration no cert file", expectErrors: true, expectedErrorSubString: "cannot specify key without cert", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -867,13 +869,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "/cluster-signing-legacy-unknown/key-file", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions LegacyUnknownSignerConfiguration no key file", expectErrors: true, expectedErrorSubString: "cannot specify cert without key", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "", ClusterSigningKeyFile: "", @@ -895,13 +897,13 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "", }, }, - }).Validate, + }, }, { name: "CSRSigningControllerOptions specific file set along with cluster single signing file", expectErrors: true, expectedErrorSubString: "cannot specify --cluster-signing-{cert,key}-file and other --cluster-signing-*-file flags at the same time", - validate: (&CSRSigningControllerOptions{ + options: &CSRSigningControllerOptions{ &csrsigningconfig.CSRSigningControllerConfiguration{ ClusterSigningCertFile: "/cluster-signing-cert-file", ClusterSigningKeyFile: "/cluster-signing-key-file", @@ -923,111 +925,111 @@ func TestValidateControllersOptions(t *testing.T) { KeyFile: "", }, }, - }).Validate, + }, }, { name: "EndpointSliceControllerOptions ConcurrentServiceEndpointSyncs lower than minConcurrentServiceEndpointSyncs (1)", expectErrors: true, expectedErrorSubString: "concurrent-service-endpoint-syncs must not be less than 1", - validate: (&EndpointSliceControllerOptions{ + options: &EndpointSliceControllerOptions{ &endpointsliceconfig.EndpointSliceControllerConfiguration{ ConcurrentServiceEndpointSyncs: 0, MaxEndpointsPerSlice: 200, }, - }).Validate, + }, }, { name: "EndpointSliceControllerOptions ConcurrentServiceEndpointSyncs greater than maxConcurrentServiceEndpointSyncs (50)", expectErrors: true, expectedErrorSubString: "concurrent-service-endpoint-syncs must not be more than 50", - validate: (&EndpointSliceControllerOptions{ + options: &EndpointSliceControllerOptions{ &endpointsliceconfig.EndpointSliceControllerConfiguration{ ConcurrentServiceEndpointSyncs: 51, MaxEndpointsPerSlice: 200, }, - }).Validate, + }, }, { name: "EndpointSliceControllerOptions MaxEndpointsPerSlice lower than minMaxEndpointsPerSlice (1)", expectErrors: true, expectedErrorSubString: "max-endpoints-per-slice must not be less than 1", - validate: (&EndpointSliceControllerOptions{ + options: &EndpointSliceControllerOptions{ &endpointsliceconfig.EndpointSliceControllerConfiguration{ ConcurrentServiceEndpointSyncs: 10, MaxEndpointsPerSlice: 0, }, - }).Validate, + }, }, { name: "EndpointSliceControllerOptions MaxEndpointsPerSlice greater than maxMaxEndpointsPerSlice (1000)", expectErrors: true, expectedErrorSubString: "max-endpoints-per-slice must not be more than 1000", - validate: (&EndpointSliceControllerOptions{ + options: &EndpointSliceControllerOptions{ &endpointsliceconfig.EndpointSliceControllerConfiguration{ ConcurrentServiceEndpointSyncs: 10, MaxEndpointsPerSlice: 1001, }, - }).Validate, + }, }, { name: "EndpointSliceMirroringControllerOptions MirroringConcurrentServiceEndpointSyncs lower than mirroringMinConcurrentServiceEndpointSyncs (1)", expectErrors: true, expectedErrorSubString: "mirroring-concurrent-service-endpoint-syncs must not be less than 1", - validate: (&EndpointSliceMirroringControllerOptions{ + options: &EndpointSliceMirroringControllerOptions{ &endpointslicemirroringconfig.EndpointSliceMirroringControllerConfiguration{ MirroringConcurrentServiceEndpointSyncs: 0, MirroringMaxEndpointsPerSubset: 100, }, - }).Validate, + }, }, { name: "EndpointSliceMirroringControllerOptions MirroringConcurrentServiceEndpointSyncs greater than mirroringMaxConcurrentServiceEndpointSyncs (50)", expectErrors: true, expectedErrorSubString: "mirroring-concurrent-service-endpoint-syncs must not be more than 50", - validate: (&EndpointSliceMirroringControllerOptions{ + options: &EndpointSliceMirroringControllerOptions{ &endpointslicemirroringconfig.EndpointSliceMirroringControllerConfiguration{ MirroringConcurrentServiceEndpointSyncs: 51, MirroringMaxEndpointsPerSubset: 100, }, - }).Validate, + }, }, { name: "EndpointSliceMirroringControllerOptions MirroringMaxEndpointsPerSubset lower than mirroringMinMaxEndpointsPerSubset (1)", expectErrors: true, expectedErrorSubString: "mirroring-max-endpoints-per-subset must not be less than 1", - validate: (&EndpointSliceMirroringControllerOptions{ + options: &EndpointSliceMirroringControllerOptions{ &endpointslicemirroringconfig.EndpointSliceMirroringControllerConfiguration{ MirroringConcurrentServiceEndpointSyncs: 10, MirroringMaxEndpointsPerSubset: 0, }, - }).Validate, + }, }, { name: "EndpointSliceMirroringControllerOptions MirroringMaxEndpointsPerSubset greater than mirroringMaxMaxEndpointsPerSubset (1000)", expectErrors: true, expectedErrorSubString: "mirroring-max-endpoints-per-subset must not be more than 1000", - validate: (&EndpointSliceMirroringControllerOptions{ + options: &EndpointSliceMirroringControllerOptions{ &endpointslicemirroringconfig.EndpointSliceMirroringControllerConfiguration{ MirroringConcurrentServiceEndpointSyncs: 10, MirroringMaxEndpointsPerSubset: 1001, }, - }).Validate, + }, }, { name: "EphemeralVolumeControllerOptions ConcurrentEphemeralVolumeSyncs equal 0", expectErrors: true, expectedErrorSubString: "concurrent-ephemeralvolume-syncs must be greater than 0", - validate: (&EphemeralVolumeControllerOptions{ + options: &EphemeralVolumeControllerOptions{ &ephemeralvolumeconfig.EphemeralVolumeControllerConfiguration{ ConcurrentEphemeralVolumeSyncs: 0, }, - }).Validate, + }, }, { name: "HPAControllerOptions ConcurrentHorizontalPodAutoscalerSyncs equal 0", expectErrors: true, expectedErrorSubString: "concurrent-horizontal-pod-autoscaler-syncs must be greater than 0", - validate: (&HPAControllerOptions{ + options: &HPAControllerOptions{ &poautosclerconfig.HPAControllerConfiguration{ ConcurrentHorizontalPodAutoscalerSyncs: 0, HorizontalPodAutoscalerSyncPeriod: metav1.Duration{Duration: 45 * time.Second}, @@ -1038,99 +1040,99 @@ func TestValidateControllersOptions(t *testing.T) { HorizontalPodAutoscalerInitialReadinessDelay: metav1.Duration{Duration: 50 * time.Second}, HorizontalPodAutoscalerTolerance: 0.1, }, - }).Validate, + }, }, { name: "NodeIPAMControllerOptions service cluster ip range more than two entries", expectErrors: true, expectedErrorSubString: "--service-cluster-ip-range can not contain more than two entries", - validate: (&NodeIPAMControllerOptions{ + options: &NodeIPAMControllerOptions{ &nodeipamconfig.NodeIPAMControllerConfiguration{ ServiceCIDR: "10.0.0.0/16,244.0.0.0/16,3000::/108", NodeCIDRMaskSize: 48, NodeCIDRMaskSizeIPv4: 48, NodeCIDRMaskSizeIPv6: 108, }, - }).Validate, + }, }, { name: "StatefulSetControllerOptions ConcurrentStatefulSetSyncs equal 0", expectErrors: true, expectedErrorSubString: "concurrent-statefulset-syncs must be greater than 0", - validate: (&StatefulSetControllerOptions{ + options: &StatefulSetControllerOptions{ &statefulsetconfig.StatefulSetControllerConfiguration{ ConcurrentStatefulSetSyncs: 0, }, - }).Validate, + }, }, { name: "JobControllerOptions ConcurrentJobSyncs equal 0", expectErrors: true, expectedErrorSubString: "concurrent-job-syncs must be greater than 0", - validate: (&JobControllerOptions{ + options: &JobControllerOptions{ &jobconfig.JobControllerConfiguration{ ConcurrentJobSyncs: 0, }, - }).Validate, + }, }, { name: "CronJobControllerOptions ConcurrentCronJobSyncs equal 0", expectErrors: true, expectedErrorSubString: "concurrent-cron-job-syncs must be greater than 0", - validate: (&CronJobControllerOptions{ + options: &CronJobControllerOptions{ &cronjobconfig.CronJobControllerConfiguration{ ConcurrentCronJobSyncs: 0, }, - }).Validate, + }, }, /* empty errs */ { name: "CronJobControllerOptions", expectErrors: false, - validate: (&CronJobControllerOptions{ + options: &CronJobControllerOptions{ &cronjobconfig.CronJobControllerConfiguration{ ConcurrentCronJobSyncs: 10, }, - }).Validate, + }, }, { name: "DaemonSetControllerOptions", expectErrors: false, - validate: (&DaemonSetControllerOptions{ + options: &DaemonSetControllerOptions{ &daemonconfig.DaemonSetControllerConfiguration{ ConcurrentDaemonSetSyncs: 2, }, - }).Validate, + }, }, { name: "DeploymentControllerOptions", expectErrors: false, - validate: (&DeploymentControllerOptions{ + options: &DeploymentControllerOptions{ &deploymentconfig.DeploymentControllerConfiguration{ ConcurrentDeploymentSyncs: 10, }, - }).Validate, + }, }, { name: "DeprecatedControllerOptions", expectErrors: false, - validate: (&DeprecatedControllerOptions{ + options: &DeprecatedControllerOptions{ &kubectrlmgrconfig.DeprecatedControllerConfiguration{}, - }).Validate, + }, }, { name: "EndpointControllerOptions", expectErrors: false, - validate: (&EndpointControllerOptions{ + options: &EndpointControllerOptions{ &endpointconfig.EndpointControllerConfiguration{ ConcurrentEndpointSyncs: 10, }, - }).Validate, + }, }, { name: "GarbageCollectorControllerOptions", expectErrors: false, - validate: (&GarbageCollectorControllerOptions{ + options: &GarbageCollectorControllerOptions{ &garbagecollectorconfig.GarbageCollectorControllerConfiguration{ ConcurrentGCSyncs: 30, GCIgnoredResources: []garbagecollectorconfig.GroupResource{ @@ -1139,31 +1141,31 @@ func TestValidateControllersOptions(t *testing.T) { }, EnableGarbageCollector: false, }, - }).Validate, + }, }, { name: "JobControllerOptions", expectErrors: false, - validate: (&JobControllerOptions{ + options: &JobControllerOptions{ &jobconfig.JobControllerConfiguration{ ConcurrentJobSyncs: 10, }, - }).Validate, + }, }, { name: "NamespaceControllerOptions", expectErrors: false, - validate: (&NamespaceControllerOptions{ + options: &NamespaceControllerOptions{ &namespaceconfig.NamespaceControllerConfiguration{ NamespaceSyncPeriod: metav1.Duration{Duration: 10 * time.Minute}, ConcurrentNamespaceSyncs: 20, }, - }).Validate, + }, }, { name: "NodeLifecycleControllerOptions", expectErrors: false, - validate: (&NodeLifecycleControllerOptions{ + options: &NodeLifecycleControllerOptions{ &nodelifecycleconfig.NodeLifecycleControllerConfiguration{ NodeEvictionRate: 0.2, SecondaryNodeEvictionRate: 0.05, @@ -1172,78 +1174,78 @@ func TestValidateControllersOptions(t *testing.T) { LargeClusterSizeThreshold: 100, UnhealthyZoneThreshold: 0.6, }, - }).Validate, + }, }, { name: "PodGCControllerOptions", expectErrors: false, - validate: (&PodGCControllerOptions{ + options: &PodGCControllerOptions{ &podgcconfig.PodGCControllerConfiguration{ TerminatedPodGCThreshold: 12000, }, - }).Validate, + }, }, { name: "ReplicaSetControllerOptions", expectErrors: false, - validate: (&ReplicaSetControllerOptions{ + options: &ReplicaSetControllerOptions{ &replicasetconfig.ReplicaSetControllerConfiguration{ ConcurrentRSSyncs: 10, }, - }).Validate, + }, }, { name: "ReplicationControllerOptions", expectErrors: false, - validate: (&ReplicationControllerOptions{ + options: &ReplicationControllerOptions{ &replicationconfig.ReplicationControllerConfiguration{ ConcurrentRCSyncs: 10, }, - }).Validate, + }, }, { name: "ResourceQuotaControllerOptions", expectErrors: false, - validate: (&ResourceQuotaControllerOptions{ + options: &ResourceQuotaControllerOptions{ &resourcequotaconfig.ResourceQuotaControllerConfiguration{ ResourceQuotaSyncPeriod: metav1.Duration{Duration: 10 * time.Minute}, ConcurrentResourceQuotaSyncs: 10, }, - }).Validate, + }, }, { name: "SAControllerOptions", expectErrors: false, - validate: (&SAControllerOptions{ + options: &SAControllerOptions{ &serviceaccountconfig.SAControllerConfiguration{ ServiceAccountKeyFile: "/service-account-private-key", ConcurrentSATokenSyncs: 10, }, - }).Validate, + }, }, { name: "LegacySATokenCleanerOptions", expectErrors: false, - validate: (&LegacySATokenCleanerOptions{ + options: &LegacySATokenCleanerOptions{ &serviceaccountconfig.LegacySATokenCleanerConfiguration{ CleanUpPeriod: metav1.Duration{Duration: 24 * 365 * time.Hour}, }, - }).Validate, + }, }, { name: "TTLAfterFinishedControllerOptions", expectErrors: false, - validate: (&TTLAfterFinishedControllerOptions{ + options: &TTLAfterFinishedControllerOptions{ &ttlafterfinishedconfig.TTLAfterFinishedControllerConfiguration{ ConcurrentTTLSyncs: 8, }, - }).Validate, + }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - errs := tc.validate() + errs := tc.options.Validate() if len(errs) > 0 && !tc.expectErrors { t.Errorf("expected no errors, errors found %+v", errs) }