Merge pull request #123613 from aojea/revert_ccm

Revert "[cloud-provider] require providerID to initialize node"

staging/src/k8s.io/cloud-provider/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

staging/src/k8s.io/cloud-provider/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"
@@ -53,8 +50,6 @@ func Test_syncNode(t *testing.T) {
 		fakeCloud    *fakecloud.Cloud
 		existingNode *v1.Node
 		updatedNode  *v1.Node
-		optionalProviderID bool
-		expectedErr        bool
 	}{
 		{
 			name: "node initialized with provider ID",
@@ -551,7 +546,6 @@ func Test_syncNode(t *testing.T) {
 		},
 		{
 			name: "provided node IP address is not valid",
-			expectedErr: true,
 			fakeCloud: &fakecloud.Cloud{
 				EnableInstancesV2: false,
 				Addresses: []v1.NodeAddress{
@@ -650,7 +644,6 @@ func Test_syncNode(t *testing.T) {
 		},
 		{
 			name: "provided node IP address is not present",
-			expectedErr: true,
 			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",
+			name: "provider ID not implemented",
-			optionalProviderID: true,
 			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{
@@ -1715,7 +1642,6 @@ func Test_syncNode(t *testing.T) {
 		},
 		{
 			name: "[instanceV2] provider ID not implemented",
-			optionalProviderID: true,
 			fakeCloud: &fakecloud.Cloud{
 				EnableInstancesV2: true,
 				InstanceTypes:     map[types.NodeName]string{},
@@ -1773,7 +1699,6 @@ func Test_syncNode(t *testing.T) {
 		},
 		{
 			name: "[instanceV2] error getting InstanceMetadata",
-			expectedErr: true,
 			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)
+			cloudNodeController.syncNode(context.TODO(), test.existingNode.Name)
-			if (err != nil) != test.expectedErr {
-				t.Fatalf("error got: %v expected: %v", err, test.expectedErr)
-			}
 
 			updatedNode, err := clientset.CoreV1().Nodes().Get(context.TODO(), test.existingNode.Name, metav1.GetOptions{})
 			if err != nil {

staging/src/k8s.io/controller-manager/pkg/features/kube_features.go

@@ -47,13 +47,6 @@ const (
 	// `alpha.kubernetes.io/provided-node-ip` annotation
 	CloudDualStackNodeIPs featuregate.Feature = "CloudDualStackNodeIPs"
 
-	// owner: @aojea
-	// Deprecated: v1.30
-	// issue: https://issues.k8s.io/123024
-	//
-	// If enabled, the ProviderID field is not required for the node initialization.
-	OptionalProviderID featuregate.Feature = "OptionalProviderID"
-
 	// owner: @alexanderConstantinescu
 	// kep: http://kep.k8s.io/3458
 	// beta: v1.27
@@ -73,6 +66,5 @@ func SetupCurrentKubernetesSpecificFeatureGates(featuregates featuregate.Mutable
 var cloudPublicFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	CloudControllerManagerWebhook: {Default: false, PreRelease: featuregate.Alpha},
 	CloudDualStackNodeIPs:         {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32
-	OptionalProviderID:            {Default: false, PreRelease: featuregate.Deprecated},             // remove after 1.31
 	StableLoadBalancerNodeSet:     {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.30, remove in 1.31
 }