From 1f8996fbabf9b4fbbec56439e1624cf823ee97dd Mon Sep 17 00:00:00 2001 From: Kamil Krzywicki <12999736+camaeel@users.noreply.github.com> Date: Fri, 3 Feb 2023 14:04:45 +0100 Subject: [PATCH 1/5] additional test + upate provideId --- pkg/spotdiscovery/awsspotdiscovery_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/spotdiscovery/awsspotdiscovery_test.go b/pkg/spotdiscovery/awsspotdiscovery_test.go index 7367130..6678100 100644 --- a/pkg/spotdiscovery/awsspotdiscovery_test.go +++ b/pkg/spotdiscovery/awsspotdiscovery_test.go @@ -14,7 +14,7 @@ var WorkerNode = &v1.Node{ Name: "test-worker-node", }, Spec: v1.NodeSpec{ - ProviderID: "aws:///eu-central-1/i-123qwe123", + ProviderID: "aws:///eu-central-1c/i-123qwe123", }, } var SpotWorkerNode = &v1.Node{ @@ -22,7 +22,7 @@ var SpotWorkerNode = &v1.Node{ Name: "test-spot-node", }, Spec: v1.NodeSpec{ - ProviderID: "aws:///eu-central-1/i-123asd132", + ProviderID: "aws:///eu-central-1b/i-123asd132", }, } var UnManagedNode = &v1.Node{ @@ -85,3 +85,11 @@ func (c *MockEC2Client) DescribeSpotInstanceRequests(in *ec2.DescribeSpotInstanc SpotInstanceRequests: spotInstances, }, nil } + +func TestReceiveInstanceID(t *testing.T) { + result := receiveInstanceID(WorkerNode) + expectedInstanceId := "i-123qwe123" + if *result != expectedInstanceId { + t.Errorf("Expected instanceId: %s, got: %s", expectedInstanceId, *result) + } +} From 18505bccda9a8787be9a8a926259f52f1b30ff4b Mon Sep 17 00:00:00 2001 From: Kamil Krzywicki <12999736+camaeel@users.noreply.github.com> Date: Fri, 3 Feb 2023 15:02:56 +0100 Subject: [PATCH 2/5] check if node has been initialized --- pkg/controller/controller.go | 18 ++++++++- pkg/controller/controller_test.go | 63 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index d0b9a30..8a35d1a 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -37,6 +37,7 @@ const ( NodeRoleSpotControlPlaneLabel = "node-role.kubernetes.io/spot-control-plane" NodeRoleWorkerLabel = "node-role.kubernetes.io/worker" NodeRoleSpotWorkerLabel = "node-role.kubernetes.io/spot-worker" + NodeUninitialziedTaint = "node.cloudprovider.kubernetes.io/uninitialized" ) func NewNodeController(client kubernetes.Interface, spotInstanceDiscovery spotdiscovery.SpotDiscoveryInterface, excludeLoadBalancing bool, includeAlphaLabel bool, excludeEviction bool, controlPlaneTaint string, controlPlaneLegacyLabel bool, customRoleLabel string) NodeController { @@ -77,7 +78,11 @@ func (c NodeController) handler(obj interface{}) { return } log.Debugf("Received handler event for node %s", node.Name) - c.markNode(node) + if c.isNodeInitialized(node) { + c.markNode(node) + } else { + log.Warnf("Node %s was not yet initialzied by cloud controller.", node.Name) + } } func (c NodeController) markNode(node *v1.Node) { @@ -168,7 +173,7 @@ func addControlPlaneLabels(node *v1.Node, includeAlphaLabel bool, excludeLoadBal } } -//Deprecated. Will be removed in future release +// Deprecated. Will be removed in future release func isAlreadyMarkedMaster(node *v1.Node) bool { if node.Labels != nil { if _, ok := node.Labels[NodeRoleMasterLabel]; ok { @@ -237,3 +242,12 @@ func (c NodeController) isWorkerNode(node *v1.Node) bool { func customRoleLabel(role string) string { return fmt.Sprintf("node-role.kubernetes.io/%s", role) } + +func (c NodeController) isNodeInitialized(node *v1.Node) bool { + for i := range node.Spec.Taints { + if node.Spec.Taints[i].Key == NodeUninitialziedTaint { + return false + } + } + return true +} diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 0ab684d..3bf646f 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -374,3 +374,66 @@ func TestHandlerShouldNotSetCustomRoleIfLabelNotPresent(t *testing.T) { t.Errorf("Expected no label %s on node %s, but was assigned", NodeRoleMasterLabel, "test-worker-node") } } + +func TestIsNodeInitializedTrue(t *testing.T) { + var initializedNode = &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-spot-control-plane-node", + }, + Spec: v1.NodeSpec{ + ProviderID: "aws:///eu-central-1/i-123uzu123", + Taints: []v1.Taint{ + { + Key: NodeRoleControlPlaneLabel, + Effect: "NoSchedule", + }, + { + Key: "dumm-taint", + Value: "true", + Effect: "NoSchedule", + }, + }, + }, + } + clientset := fake.NewSimpleClientset(initializedNode) + testingMockDiscovery := TestingMockDiscovery{} + c := NewNodeController(clientset, testingMockDiscovery, false, false, false, NodeRoleControlPlaneLabel, false, "") + + result := c.isNodeInitialized(initializedNode) + expected := true + if result != expected { + t.Errorf("Node is not initialized, but should be.") + } +} + +func TestIsNodeInitializedFalse(t *testing.T) { + var uninitializedNode = &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-spot-control-plane-node", + }, + Spec: v1.NodeSpec{ + ProviderID: "aws:///eu-central-1/i-123uzu123", + Taints: []v1.Taint{ + { + Key: NodeRoleControlPlaneLabel, + Effect: "NoSchedule", + }, + { + Key: NodeUninitialziedTaint, + Value: "true", + Effect: "NoSchedule", + }, + }, + }, + } + + clientset := fake.NewSimpleClientset(uninitializedNode) + testingMockDiscovery := TestingMockDiscovery{} + c := NewNodeController(clientset, testingMockDiscovery, false, false, false, NodeRoleControlPlaneLabel, false, "") + + result := c.isNodeInitialized(uninitializedNode) + expected := false + if result != expected { + t.Errorf("Node is initialized, but shouldn't be.") + } +} From 552b5efd234dffb0960db9f308e2a117bc48678c Mon Sep 17 00:00:00 2001 From: Kamil Krzywicki <12999736+camaeel@users.noreply.github.com> Date: Fri, 3 Feb 2023 15:14:29 +0100 Subject: [PATCH 3/5] additional test --- pkg/controller/controller_test.go | 58 ++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 3bf646f..546def5 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -99,6 +100,25 @@ var UnManagedNode = &v1.Node{ }, Spec: v1.NodeSpec{}, } +var UninitializedNode = &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-uninitialized-control-plane-node", + }, + Spec: v1.NodeSpec{ + ProviderID: "aws:///eu-central-1a/i-123uzu123", + Taints: []v1.Taint{ + { + Key: NodeRoleControlPlaneLabel, + Effect: "NoSchedule", + }, + { + Key: NodeUninitialziedTaint, + Value: "true", + Effect: "NoSchedule", + }, + }, + }, +} func TestHandlerShouldSetNodeRoleMasterAndControlPlaneForMaster(t *testing.T) { clientset := fake.NewSimpleClientset(MasterNode) @@ -407,33 +427,29 @@ func TestIsNodeInitializedTrue(t *testing.T) { } func TestIsNodeInitializedFalse(t *testing.T) { - var uninitializedNode = &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-spot-control-plane-node", - }, - Spec: v1.NodeSpec{ - ProviderID: "aws:///eu-central-1/i-123uzu123", - Taints: []v1.Taint{ - { - Key: NodeRoleControlPlaneLabel, - Effect: "NoSchedule", - }, - { - Key: NodeUninitialziedTaint, - Value: "true", - Effect: "NoSchedule", - }, - }, - }, - } - clientset := fake.NewSimpleClientset(uninitializedNode) + clientset := fake.NewSimpleClientset(UninitializedNode) testingMockDiscovery := TestingMockDiscovery{} c := NewNodeController(clientset, testingMockDiscovery, false, false, false, NodeRoleControlPlaneLabel, false, "") - result := c.isNodeInitialized(uninitializedNode) + result := c.isNodeInitialized(UninitializedNode) expected := false if result != expected { t.Errorf("Node is initialized, but shouldn't be.") } } + +func TestHandlerShouldNotSetLabelIfNodeUninitialized(t *testing.T) { + clientset := fake.NewSimpleClientset(UninitializedNode) + testingMockDiscovery := TestingMockDiscovery{} + + c := NewNodeController(clientset, testingMockDiscovery, false, false, false, NodeRoleControlPlaneLabel, false, "") + c.handler(UninitializedNode) + + foundNode, _ := clientset.CoreV1().Nodes().Get(context.TODO(), "test-uninitialized-control-plane-node", metav1.GetOptions{}) + for k, v := range foundNode.Labels { + if strings.Contains(k, "node-role.kubernetes.io/") { + t.Errorf("Unexpected label %s on node %s, but shouldn't be assigned", v, "test-uninitialized-control-plane-node") + } + } +} From 62c6c85b9a82406d4a513033de1f858edeadfa10 Mon Sep 17 00:00:00 2001 From: Kamil Krzywicki <12999736+camaeel@users.noreply.github.com> Date: Fri, 3 Feb 2023 15:16:20 +0100 Subject: [PATCH 4/5] bump golang.org/x/net --- go.mod | 8 ++++---- go.sum | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index a55f9a5..7987e9c 100644 --- a/go.mod +++ b/go.mod @@ -29,11 +29,11 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - golang.org/x/net v0.1.0 // indirect + golang.org/x/net v0.5.0 // indirect golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect - golang.org/x/sys v0.1.0 // indirect - golang.org/x/term v0.1.0 // indirect - golang.org/x/text v0.4.0 // indirect + golang.org/x/sys v0.4.0 // indirect + golang.org/x/term v0.4.0 // indirect + golang.org/x/text v0.6.0 // indirect golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.27.1 // indirect diff --git a/go.sum b/go.sum index 69ecf57..5a854ad 100644 --- a/go.sum +++ b/go.sum @@ -338,6 +338,8 @@ golang.org/x/net v0.0.0-20211209124913-491a49abca63/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.1.0 h1:hZ/3BUoy5aId7sCpA/Tc5lt8DkFgdVS2onTpJsZ/fl0= golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= +golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw= +golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -413,11 +415,13 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.1.0 h1:g6Z6vPFA9dYBAF7DWcH6sCcOntplXsDKcliusYijMlw= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -429,6 +433,7 @@ golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.6.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= From 31970c5e8115eece81aa829778fd5f1f93b616ba Mon Sep 17 00:00:00 2001 From: Kamil Krzywicki <12999736+camaeel@users.noreply.github.com> Date: Fri, 3 Feb 2023 15:22:48 +0100 Subject: [PATCH 5/5] fixes --- go.sum | 7 +++---- pkg/controller/controller_test.go | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/go.sum b/go.sum index 5a854ad..1669774 100644 --- a/go.sum +++ b/go.sum @@ -336,7 +336,6 @@ golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLd golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20211209124913-491a49abca63/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.1.0 h1:hZ/3BUoy5aId7sCpA/Tc5lt8DkFgdVS2onTpJsZ/fl0= golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw= golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws= @@ -413,14 +412,14 @@ golang.org/x/sys v0.0.0-20210831042530-f4d43177bf5e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18= golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.1.0 h1:g6Z6vPFA9dYBAF7DWcH6sCcOntplXsDKcliusYijMlw= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.4.0 h1:O7UWfv5+A2qiuulQk30kVinPoMtoIPeVaKLEgLpVkvg= golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -431,8 +430,8 @@ golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.6.0 h1:3XmdazWV+ubf7QgHSTWeykHOci5oeekaGJBLkrkaw4k= golang.org/x/text v0.6.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 546def5..349b884 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -447,9 +447,9 @@ func TestHandlerShouldNotSetLabelIfNodeUninitialized(t *testing.T) { c.handler(UninitializedNode) foundNode, _ := clientset.CoreV1().Nodes().Get(context.TODO(), "test-uninitialized-control-plane-node", metav1.GetOptions{}) - for k, v := range foundNode.Labels { + for k, _ := range foundNode.Labels { if strings.Contains(k, "node-role.kubernetes.io/") { - t.Errorf("Unexpected label %s on node %s, but shouldn't be assigned", v, "test-uninitialized-control-plane-node") + t.Errorf("Unexpected label %s on node %s, but shouldn't be assigned", k, "test-uninitialized-control-plane-node") } } }