Merge pull request #34 from jkyros/nodeclaim-labels-disruption-flapping

Pass labels from machineDeployment to NodeClaim to avoid continuous drift

Author: k8s-ci-robot

pkg/cloudprovider/cloudprovider.go

@@ -382,7 +382,10 @@ func (c *CloudProvider) machineToNodeClaim(ctx context.Context, machine *capiv1b
 		return nil, fmt.Errorf("unable to convert Machine %q to a NodeClaim, no memory capacity found on MachineDeployment %q", machine.GetName(), machineDeployment.Name)
 	}
 
-	// TODO (elmiko) add labels, and taints
+	// Set NodeClaim labels from the MachineDeployment
+	nodeClaim.Labels = nodeLabelsFromMachineDeployment(machineDeployment)
+
+	// TODO (elmiko) add taints
 
 	nodeClaim.Status.Capacity = capacity
 
@@ -532,7 +535,10 @@ func createNodeClaimFromMachineDeployment(machineDeployment *capiv1beta1.Machine
 	nodeClaim.Status.Capacity = instanceType.Capacity
 	nodeClaim.Status.Allocatable = instanceType.Allocatable()
 
-	// TODO (elmiko) we might need to also convey the labels and annotations on to the NodeClaim
+	// Set NodeClaim labels from the MachineDeployment
+	nodeClaim.Labels = nodeLabelsFromMachineDeployment(machineDeployment)
+
+	// TODO (elmiko) add taints
 
 	return nodeClaim
 }
@@ -593,9 +599,15 @@ func machineDeploymentToInstanceType(machineDeployment *capiv1beta1.MachineDeplo
 	}
 
 	instanceType.Offerings = offerings
-	// TODO (elmiko) this may not be correct given the code comment in the InstanceType struct about the name corresponding
+
+	// TODO (elmiko) find a better way to learn the instance type. The instance name needs to be the same
+	// as the well-known `node.kubernetes.io/instance-type` that will be on resulting nodes. Usually, a
+	// cloud controller, or similar, mechanism is used to apply this label.
+	// For now, we check the labels we know about from the MachineDeployment, if the instance type label
+	// is not there, leave it blank.
 	// to the v1.LabelInstanceTypeStable. if karpenter expects this to match the node, then we need to get this value through capi.
-	instanceType.Name = machineDeployment.Name
+	// TODO (jkyros) Add a test case to test this behavior
+	instanceType.Name = labels[corev1.LabelInstanceTypeStable]
 
 	// TODO (elmiko) add the proper overhead information, not sure where we will harvest this information.
 	// perhaps it needs to be a configurable option somewhere.

pkg/cloudprovider/cloudprovider_test.go

@@ -349,7 +349,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		Expect(instanceType.Capacity).Should(HaveKeyWithValue(corev1.ResourceCPU, resource.MustParse("1")))
 		Expect(instanceType.Capacity).Should(HaveKeyWithValue(corev1.ResourceMemory, resource.MustParse("16Gi")))
 		Expect(instanceType.Capacity).Should(HaveKeyWithValue(corev1.ResourceName("nvidia.com/gpu"), resource.MustParse("1")))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 
 	It("adds nothing to requirements when no managed labels or scale from zero annotations are present", func() {
@@ -358,7 +358,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		instanceType := machineDeploymentToInstanceType(machineDeployment)
 
 		Expect(instanceType.Requirements).To(HaveLen(0))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 
 	It("adds labels to the requirements from the Cluster API propagation rules", func() {
@@ -381,7 +381,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		Expect(instanceType.Requirements).Should(HaveKey("prefixed.node-restriction.kubernetes.io/some-other-thing"))
 		Expect(instanceType.Requirements).Should(HaveKey("node.cluster.x-k8s.io/another-thing"))
 		Expect(instanceType.Requirements).Should(HaveKey("prefixed.node.cluster.x-k8s.io/another-thing"))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 
 	It("adds labels to the requirements from the scale from zero annotations", func() {
@@ -395,7 +395,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		Expect(instanceType.Requirements).To(HaveLen(2))
 		Expect(instanceType.Requirements).Should(HaveKey(corev1.LabelTopologyZone))
 		Expect(instanceType.Requirements).Should(HaveKey(InstanceSizeLabelKey))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 
 	It("adds labels to the requirements from the propagation rules and the scale from zero annotations", func() {
@@ -424,7 +424,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		Expect(instanceType.Requirements).Should(HaveKey("prefixed.node-restriction.kubernetes.io/some-other-thing"))
 		Expect(instanceType.Requirements).Should(HaveKey("node.cluster.x-k8s.io/another-thing"))
 		Expect(instanceType.Requirements).Should(HaveKey("prefixed.node.cluster.x-k8s.io/another-thing"))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 
 	It("adds a single available on-demand offering with price 0 and empty zone", func() {
@@ -438,7 +438,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		Expect(offering.Requirements[karpv1.CapacityTypeLabelKey].Values()).Should(ContainElement(karpv1.CapacityTypeOnDemand))
 		Expect(offering.Requirements).Should(HaveKey(corev1.LabelTopologyZone))
 		Expect(offering.Requirements[corev1.LabelTopologyZone].Values()).Should(ContainElement(""))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 
 	It("adds the correct zone to offering when the well known zone label is present", func() {
@@ -453,7 +453,7 @@ var _ = Describe("machineDeploymentToInstanceType function", func() {
 		offering := instanceType.Offerings[0]
 		Expect(offering.Requirements).Should(HaveKey(corev1.LabelTopologyZone))
 		Expect(offering.Requirements[corev1.LabelTopologyZone].Values()).Should(ContainElement(zone))
-		Expect(instanceType.Name).To(Equal(machineDeployment.Name))
+		Expect(instanceType.MachineDeploymentName).To(Equal(machineDeployment.Name))
 	})
 })