Skip to content

Commit c10421f

Browse files
authored
rename windows flags (#371)
1 parent 18352ea commit c10421f

File tree

4 files changed

+198
-34
lines changed

4 files changed

+198
-34
lines changed

pkg/config/loader.go

+9
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,17 @@ func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTa
9292
}
9393

9494
warmIPTargetStr, foundWarmIP := vpcCniConfigMap.Data[WarmIPTarget]
95+
if !foundWarmIP {
96+
warmIPTargetStr, foundWarmIP = vpcCniConfigMap.Data[WinWarmIPTarget]
97+
}
9598
minIPTargetStr, foundMinIP := vpcCniConfigMap.Data[MinimumIPTarget]
99+
if !foundMinIP {
100+
minIPTargetStr, foundMinIP = vpcCniConfigMap.Data[WinMinimumIPTarget]
101+
}
96102
warmPrefixTargetStr, foundWarmPrefix := vpcCniConfigMap.Data[WarmPrefixTarget]
103+
if !foundWarmPrefix {
104+
warmPrefixTargetStr, foundWarmPrefix = vpcCniConfigMap.Data[WinWarmPrefixTarget]
105+
}
97106

98107
// If no configuration is found, return 0
99108
if !foundWarmIP && !foundMinIP && !foundWarmPrefix {

pkg/config/type.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,14 @@ const (
7373
VpcCniConfigMapName = "amazon-vpc-cni"
7474
EnableWindowsIPAMKey = "enable-windows-ipam"
7575
EnableWindowsPrefixDelegationKey = "enable-windows-prefix-delegation"
76-
WarmPrefixTarget = "warm-prefix-target"
77-
WarmIPTarget = "warm-ip-target"
78-
MinimumIPTarget = "minimum-ip-target"
76+
// TODO: we will deprecate the confusing naming of Windows flags eventually
77+
WarmPrefixTarget = "warm-prefix-target"
78+
WarmIPTarget = "warm-ip-target"
79+
MinimumIPTarget = "minimum-ip-target"
80+
// these windows prefixed flags will be used for Windows support only eventully
81+
WinWarmPrefixTarget = "windows-warm-prefix-target"
82+
WinWarmIPTarget = "windows-warm-ip-target"
83+
WinMinimumIPTarget = "windows-minimum-ip-target"
7984
// Since LeaderElectionNamespace and VpcCniConfigMapName may be different in the future
8085
KubeSystemNamespace = "kube-system"
8186
VpcCNIDaemonSetName = "aws-node"

pkg/provider/prefix/provider_test.go

+47-31
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ import (
2222
mock_ec2 "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2"
2323
mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition"
2424
mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s"
25-
"github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/pool"
26-
"github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni"
27-
"github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/worker"
25+
mock_pool "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/pool"
26+
mock_eni "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni"
27+
mock_worker "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/worker"
2828
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api"
2929
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
3030
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool"
@@ -68,6 +68,16 @@ var (
6868
},
6969
}
7070

71+
vpcCNIConfigWindows = &v1.ConfigMap{
72+
Data: map[string]string{
73+
config.EnableWindowsIPAMKey: "true",
74+
config.EnableWindowsPrefixDelegationKey: "true",
75+
config.WinWarmIPTarget: strconv.Itoa(config.IPv4PDDefaultWarmIPTargetSize),
76+
config.WinMinimumIPTarget: strconv.Itoa(config.IPv4PDDefaultMinIPTargetSize),
77+
config.WinWarmPrefixTarget: strconv.Itoa(config.IPv4PDDefaultWarmPrefixTargetSize),
78+
},
79+
}
80+
7181
node = &v1.Node{
7282
ObjectMeta: metav1.ObjectMeta{
7383
Name: nodeName,
@@ -386,23 +396,25 @@ func TestIPv4PrefixProvider_UpdateResourceCapacity_FromFromIPToPD(t *testing.T)
386396
instanceProviderAndPool: map[string]*ResourceProviderAndPool{},
387397
log: zap.New(zap.UseDevMode(true)).WithName("prefix provider"), conditions: mockConditions}
388398

389-
mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(vpcCNIConfig, nil)
390-
mockPool := mock_pool.NewMockPool(ctrl)
391-
mockManager := mock_eni.NewMockENIManager(ctrl)
392-
prefixProvider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true)
393-
mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true)
399+
for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} {
400+
mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil)
401+
mockPool := mock_pool.NewMockPool(ctrl)
402+
mockManager := mock_eni.NewMockENIManager(ctrl)
403+
prefixProvider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true)
404+
mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true)
394405

395-
job := &worker.WarmPoolJob{Operations: worker.OperationCreate}
396-
mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job)
397-
mockWorker.EXPECT().SubmitJob(job)
406+
job := &worker.WarmPoolJob{Operations: worker.OperationCreate}
407+
mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job)
408+
mockWorker.EXPECT().SubmitJob(job)
398409

399-
mockInstance.EXPECT().Name().Return(nodeName).Times(2)
400-
mockInstance.EXPECT().Type().Return(instanceType)
401-
mockInstance.EXPECT().Os().Return(config.OSWindows)
402-
mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil)
410+
mockInstance.EXPECT().Name().Return(nodeName).Times(2)
411+
mockInstance.EXPECT().Type().Return(instanceType)
412+
mockInstance.EXPECT().Os().Return(config.OSWindows)
413+
mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil)
403414

404-
err := prefixProvider.UpdateResourceCapacity(mockInstance)
405-
assert.NoError(t, err)
415+
err := prefixProvider.UpdateResourceCapacity(mockInstance)
416+
assert.NoError(t, err)
417+
}
406418
}
407419

408420
// TestIPv4PrefixProvider_UpdateResourceCapacity_FromFromPDToIP tests the warm pool is drained when PD is disabled
@@ -449,21 +461,23 @@ func TestIPv4PrefixProvider_UpdateResourceCapacity_FromPDToPD(t *testing.T) {
449461
mockPool := mock_pool.NewMockPool(ctrl)
450462
mockManager := mock_eni.NewMockENIManager(ctrl)
451463
prefixProvider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true)
452-
mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true)
453464

454-
mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(vpcCNIConfig, nil)
465+
for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} {
466+
mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true)
467+
mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil)
455468

456-
job := &worker.WarmPoolJob{Operations: worker.OperationCreate}
457-
mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job)
458-
mockWorker.EXPECT().SubmitJob(job)
469+
job := &worker.WarmPoolJob{Operations: worker.OperationCreate}
470+
mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job)
471+
mockWorker.EXPECT().SubmitJob(job)
459472

460-
mockInstance.EXPECT().Name().Return(nodeName).Times(2)
461-
mockInstance.EXPECT().Type().Return(instanceType)
462-
mockInstance.EXPECT().Os().Return(config.OSWindows)
463-
mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil)
473+
mockInstance.EXPECT().Name().Return(nodeName).Times(2)
474+
mockInstance.EXPECT().Type().Return(instanceType)
475+
mockInstance.EXPECT().Os().Return(config.OSWindows)
476+
mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil)
464477

465-
err := prefixProvider.UpdateResourceCapacity(mockInstance)
466-
assert.NoError(t, err)
478+
err := prefixProvider.UpdateResourceCapacity(mockInstance)
479+
assert.NoError(t, err)
480+
}
467481
}
468482

469483
// TestIPv4PrefixProvider_UpdateResourceCapacity_FromIPToIP tests the resource capacity is not updated when secondary IP mode stays enabled
@@ -539,10 +553,12 @@ func TestGetPDWarmPoolConfig(t *testing.T) {
539553
instanceProviderAndPool: map[string]*ResourceProviderAndPool{},
540554
log: zap.New(zap.UseDevMode(true)).WithName("prefix provider"), conditions: mockConditions}
541555

542-
mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(vpcCNIConfig, nil)
556+
for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} {
557+
mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil)
543558

544-
config := prefixProvider.getPDWarmPoolConfig()
545-
assert.Equal(t, pdWarmPoolConfig, config)
559+
config := prefixProvider.getPDWarmPoolConfig()
560+
assert.Equal(t, pdWarmPoolConfig, config)
561+
}
546562
}
547563

548564
// TestIsInstanceSupported tests that if the instance type is nitro, return true

test/integration/windows/windows_test.go

+134
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ var _ = Describe("Windows Integration Test", func() {
285285
})
286286
})
287287

288+
// TODO: remove this context when VPC CNI also updates the flag name to windows prefixed.
288289
Context("When warm-prefix-target is set to 2", Label("warm-prefix-target"), func() {
289290
BeforeEach(func() {
290291
data = map[string]string{
@@ -316,6 +317,7 @@ var _ = Describe("Windows Integration Test", func() {
316317
})
317318
})
318319

320+
// TODO: remove this context when VPC CNI also updates the flag name to windows prefixed.
319321
Context("When warm-ip-target is set to 15", Label("warm-ip-target"), func() {
320322
BeforeEach(func() {
321323
data = map[string]string{
@@ -362,6 +364,7 @@ var _ = Describe("Windows Integration Test", func() {
362364
})
363365
})
364366

367+
// TODO: remove this context when VPC CNI also updates the flag name to windows prefixed.
365368
Context("When minimum-ip-target is set to 20", Label("minimum-ip-target"), func() {
366369
BeforeEach(func() {
367370
data = map[string]string{
@@ -415,6 +418,137 @@ var _ = Describe("Windows Integration Test", func() {
415418
})
416419
})
417420

421+
Context("When windows-warm-prefix-target is set to 2", Label("windows-warm-prefix-target"), func() {
422+
BeforeEach(func() {
423+
data = map[string]string{
424+
config.EnableWindowsIPAMKey: "true",
425+
config.EnableWindowsPrefixDelegationKey: "true",
426+
config.WinWarmPrefixTarget: "2"}
427+
428+
})
429+
430+
It("two prefixes should be assigned", func() {
431+
// allow some time for previous test pod to cool down
432+
time.Sleep(bufferForCoolDown)
433+
_, prefixesBefore, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
434+
Expect(err).ToNot(HaveOccurred())
435+
Expect(len(prefixesBefore)).To(Equal(2))
436+
437+
By("creating pod and waiting for ready should have 1 new prefix assigned")
438+
// verify if ip assigned is coming from a prefix
439+
createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout)
440+
Expect(err).ToNot(HaveOccurred())
441+
verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesBefore)
442+
443+
// number of prefixes should increase by 1 since need 1 more prefix to fulfill warm-prefix-target of 2
444+
_, prefixesAfter, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
445+
Expect(err).ToNot(HaveOccurred())
446+
Expect(len(prefixesAfter) - len(prefixesBefore)).To(Equal(1))
447+
448+
err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod)
449+
Expect(err).ToNot(HaveOccurred())
450+
})
451+
})
452+
453+
Context("When windows-warm-ip-target is set to 15", Label("windows-warm-ip-target"), func() {
454+
BeforeEach(func() {
455+
data = map[string]string{
456+
config.EnableWindowsIPAMKey: "true",
457+
config.EnableWindowsPrefixDelegationKey: "true",
458+
config.WinWarmIPTarget: "15"}
459+
})
460+
It("should assign new prefix when 2nd pod is launched", func() {
461+
// allow some time for previous test pod to cool down
462+
time.Sleep(bufferForCoolDown)
463+
// before running any pod, should have 1 prefix assigned
464+
privateIPsBefore, prefixesBefore, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
465+
Expect(err).ToNot(HaveOccurred())
466+
Expect(len(prefixesBefore)).To(Equal(1))
467+
468+
By("creating 1 pod and waiting for ready should not create new prefix")
469+
// verify if ip assigned is coming from a prefix
470+
createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout)
471+
Expect(err).ToNot(HaveOccurred())
472+
473+
_, prefixesAfterPod1, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
474+
Expect(err).ToNot(HaveOccurred())
475+
Expect(len(prefixesAfterPod1)).To(Equal(len(prefixesBefore)))
476+
verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesAfterPod1)
477+
478+
// launch 2nd pod to trigger a new prefix to be assigned since warm-ip-target=15
479+
By("creating 2nd pod and waiting for ready should have 1 more prefix assigned")
480+
createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod2, utils.WindowsPodsCreationTimeout)
481+
Expect(err).ToNot(HaveOccurred())
482+
verify.WindowsPodHaveResourceLimits(createdPod, true)
483+
484+
privateIPsAfter, prefixesAfterPod2, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
485+
Expect(err).ToNot(HaveOccurred())
486+
// 1 more prefix should be created to fulfill warm-ip-target=15
487+
Expect(len(prefixesAfterPod2) - len(prefixesAfterPod1)).To(Equal(1))
488+
// number of secondary ips should not change
489+
Expect(len(privateIPsBefore)).To(Equal(len(privateIPsAfter)))
490+
verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesAfterPod2)
491+
492+
err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod)
493+
Expect(err).ToNot(HaveOccurred())
494+
err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2)
495+
Expect(err).ToNot(HaveOccurred())
496+
})
497+
})
498+
499+
Context("When windows-minimum-ip-target is set to 20", Label("windows-minimum-ip-target"), func() {
500+
BeforeEach(func() {
501+
data = map[string]string{
502+
config.EnableWindowsIPAMKey: "true",
503+
config.EnableWindowsPrefixDelegationKey: "true",
504+
config.WinMinimumIPTarget: "20"}
505+
})
506+
It("should have 2 prefixes to satisfy windows-minimum-ip-target when no pods running", func() {
507+
By("adding labels to selected nodes for testing")
508+
node := windowsNodeList.Items[0]
509+
err = frameWork.NodeManager.AddLabels([]v1.Node{node}, map[string]string{podLabelKey: podLabelVal})
510+
Expect(err).ToNot(HaveOccurred())
511+
512+
// allow some time for previous test pod to cool down
513+
time.Sleep(bufferForCoolDown)
514+
// before running any pod, should have 2 prefixes assigned
515+
instanceID = manager.GetNodeInstanceID(&node)
516+
privateIPsBefore, prefixesBefore, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
517+
Expect(err).ToNot(HaveOccurred())
518+
Expect(len(prefixesBefore)).To(Equal(2))
519+
520+
By("creating 33 pods and waiting for ready should have 3 prefixes attached")
521+
deployment := manifest.NewWindowsDeploymentBuilder().
522+
Replicas(33).
523+
Container(manifest.NewWindowsContainerBuilder().Build()).
524+
PodLabel(podLabelKey, podLabelVal).
525+
NodeSelector(map[string]string{"kubernetes.io/os": "windows", podLabelKey: podLabelVal}).
526+
Build()
527+
_, err = frameWork.DeploymentManager.CreateAndWaitUntilDeploymentReady(ctx, deployment)
528+
Expect(err).ToNot(HaveOccurred())
529+
530+
_, prefixesAfterDeployment, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
531+
Expect(err).ToNot(HaveOccurred())
532+
Expect(len(prefixesAfterDeployment)).To(Equal(3))
533+
534+
By("deleting 33 pods should still have 2 prefixes attached")
535+
err = frameWork.DeploymentManager.DeleteAndWaitUntilDeploymentDeleted(ctx, deployment)
536+
Expect(err).ToNot(HaveOccurred())
537+
538+
// allow some time for previous test pods to cool down since deletion of deployment doesn't wait for pods to terminate
539+
time.Sleep(utils.WindowsPodsDeletionTimeout)
540+
privateIPsAfter, prefixesAfterDelete, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID)
541+
Expect(err).ToNot(HaveOccurred())
542+
Expect(len(prefixesAfterDelete)).To(Equal(2))
543+
// number of secondary ips should not change
544+
Expect(len(privateIPsBefore)).To(Equal(len(privateIPsAfter)))
545+
546+
By("removing labels on selected nodes for testing")
547+
err = frameWork.NodeManager.RemoveLabels([]v1.Node{node}, map[string]string{podLabelKey: podLabelVal})
548+
Expect(err).ToNot(HaveOccurred())
549+
})
550+
})
551+
418552
Context("[CANARY] When enable-windows-prefix-delegation is toggled to false", func() {
419553
BeforeEach(func() {
420554
data = map[string]string{

0 commit comments

Comments
 (0)