diff --git a/controllers/node/node_controller.go b/controllers/node/node_controller.go index 4f0f4d31..b31c04d4 100644 --- a/controllers/node/node_controller.go +++ b/controllers/node/node_controller.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" coreinformers "k8s.io/client-go/informers/core/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" @@ -45,7 +44,6 @@ import ( cloudnodeutil "k8s.io/cloud-provider/node/helpers" controllersmetrics "k8s.io/component-base/metrics/prometheus/controllers" nodeutil "k8s.io/component-helpers/node/util" - "k8s.io/controller-manager/pkg/features" "k8s.io/klog/v2" ) @@ -483,19 +481,6 @@ func (cnc *CloudNodeController) syncNode(ctx context.Context, nodeName string) e modify(newNode) } - // spec.ProviderID is required for multiple controllers, like loadbalancers, so we should not - // untaint the node until is set. Once it is set, the field is immutable, so no need to reconcile. - // We only set this value during initialization and is never reconciled, so if for some reason - // we are not able to set it, the instance will never be able to acquire it. - // Before external cloud providers were enabled by default, the field was set by the kubelet, and the - // node was created with the value. - // xref: https://issues.k8s.io/123024 - if !utilfeature.DefaultFeatureGate.Enabled(features.OptionalProviderID) { - if newNode.Spec.ProviderID == "" { - return fmt.Errorf("failed to get provider ID for node %s at cloudprovider", nodeName) - } - } - _, err = cnc.kubeClient.CoreV1().Nodes().Update(context.TODO(), newNode, metav1.UpdateOptions{}) if err != nil { return err diff --git a/controllers/node/node_controller_test.go b/controllers/node/node_controller_test.go index af8aa55a..7f5e95e0 100644 --- a/controllers/node/node_controller_test.go +++ b/controllers/node/node_controller_test.go @@ -32,15 +32,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" clienttesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" cloudprovider "k8s.io/cloud-provider" cloudproviderapi "k8s.io/cloud-provider/api" fakecloud "k8s.io/cloud-provider/fake" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/controller-manager/pkg/features" _ "k8s.io/controller-manager/pkg/features/register" "github.com/google/go-cmp/cmp" @@ -49,12 +46,10 @@ import ( func Test_syncNode(t *testing.T) { tests := []struct { - name string - fakeCloud *fakecloud.Cloud - existingNode *v1.Node - updatedNode *v1.Node - optionalProviderID bool - expectedErr bool + name string + fakeCloud *fakecloud.Cloud + existingNode *v1.Node + updatedNode *v1.Node }{ { name: "node initialized with provider ID", @@ -550,8 +545,7 @@ func Test_syncNode(t *testing.T) { }, }, { - name: "provided node IP address is not valid", - expectedErr: true, + name: "provided node IP address is not valid", fakeCloud: &fakecloud.Cloud{ EnableInstancesV2: false, Addresses: []v1.NodeAddress{ @@ -649,8 +643,7 @@ func Test_syncNode(t *testing.T) { }, }, { - name: "provided node IP address is not present", - expectedErr: true, + name: "provided node IP address is not present", fakeCloud: &fakecloud.Cloud{ EnableInstancesV2: false, Addresses: []v1.NodeAddress{ @@ -843,8 +836,7 @@ func Test_syncNode(t *testing.T) { }, }, { - name: "provider ID not implemented and optional", - optionalProviderID: true, + name: "provider ID not implemented", fakeCloud: &fakecloud.Cloud{ EnableInstancesV2: false, InstanceTypes: map[types.NodeName]string{}, @@ -900,71 +892,6 @@ func Test_syncNode(t *testing.T) { }, }, }, - { - name: "provider ID not implemented and required", - optionalProviderID: false, - expectedErr: true, - fakeCloud: &fakecloud.Cloud{ - EnableInstancesV2: false, - InstanceTypes: map[types.NodeName]string{}, - Provider: "test", - ExtID: map[types.NodeName]string{}, - ExtIDErr: map[types.NodeName]error{ - types.NodeName("node0"): cloudprovider.NotImplemented, - }, - Err: nil, - }, - existingNode: &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node0", - CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), - }, - Status: v1.NodeStatus{ - Conditions: []v1.NodeCondition{ - { - Type: v1.NodeReady, - Status: v1.ConditionUnknown, - LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), - LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), - }, - }, - }, - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - { - Key: cloudproviderapi.TaintExternalCloudProvider, - Value: "true", - Effect: v1.TaintEffectNoSchedule, - }, - }, - }, - }, - updatedNode: &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node0", - CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), - }, - Status: v1.NodeStatus{ - Conditions: []v1.NodeCondition{ - { - Type: v1.NodeReady, - Status: v1.ConditionUnknown, - LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), - LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), - }, - }, - }, - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - { - Key: cloudproviderapi.TaintExternalCloudProvider, - Value: "true", - Effect: v1.TaintEffectNoSchedule, - }, - }, - }, - }, - }, { name: "[instanceV2] node initialized with provider ID", fakeCloud: &fakecloud.Cloud{ @@ -1714,8 +1641,7 @@ func Test_syncNode(t *testing.T) { }, }, { - name: "[instanceV2] provider ID not implemented", - optionalProviderID: true, + name: "[instanceV2] provider ID not implemented", fakeCloud: &fakecloud.Cloud{ EnableInstancesV2: true, InstanceTypes: map[types.NodeName]string{}, @@ -1772,8 +1698,7 @@ func Test_syncNode(t *testing.T) { }, }, { - name: "[instanceV2] error getting InstanceMetadata", - expectedErr: true, + name: "[instanceV2] error getting InstanceMetadata", fakeCloud: &fakecloud.Cloud{ EnableInstancesV2: true, InstanceTypes: map[types.NodeName]string{}, @@ -1839,7 +1764,6 @@ func Test_syncNode(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.OptionalProviderID, test.optionalProviderID)() clientset := fake.NewSimpleClientset(test.existingNode) factory := informers.NewSharedInformerFactory(clientset, 0) @@ -1862,10 +1786,7 @@ func Test_syncNode(t *testing.T) { w := eventBroadcaster.StartLogging(klog.Infof) defer w.Stop() - err := cloudNodeController.syncNode(context.TODO(), test.existingNode.Name) - if (err != nil) != test.expectedErr { - t.Fatalf("error got: %v expected: %v", err, test.expectedErr) - } + cloudNodeController.syncNode(context.TODO(), test.existingNode.Name) updatedNode, err := clientset.CoreV1().Nodes().Get(context.TODO(), test.existingNode.Name, metav1.GetOptions{}) if err != nil {