Skip to content

Commit

Permalink
Merge pull request #123331 from aojea/ccm_update
Browse files Browse the repository at this point in the history 
CCM wait for providerID to initialize the Node object

Kubernetes-commit: 411c29c39f03687a30a8667295c61590def8fc89
  • Loading branch information
@k8s-publishing-bot
k8s-publishing-bot committed Feb 29, 2024
2 parents dd1ad5e + 57b2532 commit 123fab8
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 26 deletions.
2 changes: 2 additions & 0 deletions app/testing/testserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ func StartTestServer(ctx context.Context, customFlags []string) (result TestServ
return TestServer{}, err
}

s.Generic.LeaderElection.LeaderElect = false

cloudInitializer := func(config *config.CompletedConfig) cloudprovider.Interface {
capturedConfig = *config
// send signal to indicate the capturedConfig has been properly set
Expand Down
25 changes: 17 additions & 8 deletions controllers/node/node_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ 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"
Expand All @@ -44,6 +45,7 @@ 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"
)

Expand Down Expand Up @@ -282,11 +284,6 @@ func (cnc *CloudNodeController) UpdateNodeStatus(ctx context.Context) error {
}

cnc.updateNodeAddress(ctx, node, instanceMetadata)

err = cnc.reconcileNodeLabels(node.Name)
if err != nil {
klog.Errorf("Error reconciling node labels for node %q, err: %v", node.Name, err)
}
}

workqueue.ParallelizeUntil(ctx, int(cnc.workerCount), len(nodes), updateNodeFunc)
Expand Down Expand Up @@ -422,9 +419,8 @@ func (cnc *CloudNodeController) syncNode(ctx context.Context, nodeName string) e

cloudTaint := getCloudTaint(curNode.Spec.Taints)
if cloudTaint == nil {
// Node object received from event had the cloud taint but was outdated,
// the node has actually already been initialized, so this sync event can be ignored.
return nil
// Node object was already initialized, only need to reconcile the labels
return cnc.reconcileNodeLabels(nodeName)
}

klog.Infof("Initializing node %s with cloud provider", nodeName)
Expand Down Expand Up @@ -487,6 +483,19 @@ 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
Expand Down
99 changes: 89 additions & 10 deletions controllers/node/node_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@ 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"
Expand All @@ -46,10 +49,12 @@ import (

func Test_syncNode(t *testing.T) {
tests := []struct {
name string
fakeCloud *fakecloud.Cloud
existingNode *v1.Node
updatedNode *v1.Node
name string
fakeCloud *fakecloud.Cloud
existingNode *v1.Node
updatedNode *v1.Node
optionalProviderID bool
expectedErr bool
}{
{
name: "node initialized with provider ID",
Expand Down Expand Up @@ -545,7 +550,8 @@ func Test_syncNode(t *testing.T) {
},
},
{
name: "provided node IP address is not valid",
name: "provided node IP address is not valid",
expectedErr: true,
fakeCloud: &fakecloud.Cloud{
EnableInstancesV2: false,
Addresses: []v1.NodeAddress{
Expand Down Expand Up @@ -643,7 +649,8 @@ func Test_syncNode(t *testing.T) {
},
},
{
name: "provided node IP address is not present",
name: "provided node IP address is not present",
expectedErr: true,
fakeCloud: &fakecloud.Cloud{
EnableInstancesV2: false,
Addresses: []v1.NodeAddress{
Expand Down Expand Up @@ -836,7 +843,8 @@ func Test_syncNode(t *testing.T) {
},
},
{
name: "provider ID not implemented",
name: "provider ID not implemented and optional",
optionalProviderID: true,
fakeCloud: &fakecloud.Cloud{
EnableInstancesV2: false,
InstanceTypes: map[types.NodeName]string{},
Expand Down Expand Up @@ -892,6 +900,71 @@ 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{
Expand Down Expand Up @@ -1641,7 +1714,8 @@ func Test_syncNode(t *testing.T) {
},
},
{
name: "[instanceV2] provider ID not implemented",
name: "[instanceV2] provider ID not implemented",
optionalProviderID: true,
fakeCloud: &fakecloud.Cloud{
EnableInstancesV2: true,
InstanceTypes: map[types.NodeName]string{},
Expand Down Expand Up @@ -1698,7 +1772,8 @@ func Test_syncNode(t *testing.T) {
},
},
{
name: "[instanceV2] error getting InstanceMetadata",
name: "[instanceV2] error getting InstanceMetadata",
expectedErr: true,
fakeCloud: &fakecloud.Cloud{
EnableInstancesV2: true,
InstanceTypes: map[types.NodeName]string{},
Expand Down Expand Up @@ -1764,6 +1839,7 @@ 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)

Expand All @@ -1786,7 +1862,10 @@ func Test_syncNode(t *testing.T) {
w := eventBroadcaster.StartLogging(klog.Infof)
defer w.Stop()

cloudNodeController.syncNode(context.TODO(), test.existingNode.Name)
err := 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 {
Expand Down
8 changes: 4 additions & 4 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ require (
github.com/stretchr/testify v1.8.4
k8s.io/api v0.0.0-20240228104440-d6ea7e8f9bfc
k8s.io/apimachinery v0.0.0-20240221202133-0f2e9357997f
k8s.io/apiserver v0.0.0-20240228231835-0a2e73e99157
k8s.io/apiserver v0.0.0-20240229122720-3d757e5f422a
k8s.io/client-go v0.0.0-20240228230157-d99a76c51e14
k8s.io/component-base v0.0.0-20240227002902-6c2a49d37aa4
k8s.io/component-helpers v0.0.0-20240221203355-866cab6f8733
k8s.io/controller-manager v0.0.0-20240228234348-3c1067cedea3
k8s.io/controller-manager v0.0.0-20240229125230-05eba06abdf4
k8s.io/klog/v2 v2.120.1
k8s.io/utils v0.0.0-20230726121419-3b25d923346b
)
Expand Down Expand Up @@ -112,10 +112,10 @@ require (
replace (
k8s.io/api => k8s.io/api v0.0.0-20240228104440-d6ea7e8f9bfc
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20240221202133-0f2e9357997f
k8s.io/apiserver => k8s.io/apiserver v0.0.0-20240228231835-0a2e73e99157
k8s.io/apiserver => k8s.io/apiserver v0.0.0-20240229122720-3d757e5f422a
k8s.io/client-go => k8s.io/client-go v0.0.0-20240228230157-d99a76c51e14
k8s.io/component-base => k8s.io/component-base v0.0.0-20240227002902-6c2a49d37aa4
k8s.io/component-helpers => k8s.io/component-helpers v0.0.0-20240221203355-866cab6f8733
k8s.io/controller-manager => k8s.io/controller-manager v0.0.0-20240228234348-3c1067cedea3
k8s.io/controller-manager => k8s.io/controller-manager v0.0.0-20240229125230-05eba06abdf4
k8s.io/kms => k8s.io/kms v0.0.0-20240221203502-15393f39a6fb
)
8 changes: 4 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,16 +298,16 @@ k8s.io/api v0.0.0-20240228104440-d6ea7e8f9bfc h1:7IBpQCcIsPKGiK8R2xVKsFYdcqvt5UI
k8s.io/api v0.0.0-20240228104440-d6ea7e8f9bfc/go.mod h1:kCYq4mrNBxRaHCZeTveL6//1FveIDm3wxJZeFNKF6b8=
k8s.io/apimachinery v0.0.0-20240221202133-0f2e9357997f h1:IvhurYeUUiUsNbDhVJHvOwhgfpUoobqtGQGat2VxfcQ=
k8s.io/apimachinery v0.0.0-20240221202133-0f2e9357997f/go.mod h1:/862Kkwje5hhHGJWPKiaHuov2c6mw6uCXWikV9kOIP4=
k8s.io/apiserver v0.0.0-20240228231835-0a2e73e99157 h1:yZx/OnFmOxR0LrRqcQuccNHCPWqvmoU4joZuI15UfeY=
k8s.io/apiserver v0.0.0-20240228231835-0a2e73e99157/go.mod h1:qKJTjU6234ehYV+y9VetdPVPplO4dlbpD55dTD0cFBA=
k8s.io/apiserver v0.0.0-20240229122720-3d757e5f422a h1:40AJ12gvvbNlFB2QQtJ98lbEkWpFru/jKPwWKRot72g=
k8s.io/apiserver v0.0.0-20240229122720-3d757e5f422a/go.mod h1:qKJTjU6234ehYV+y9VetdPVPplO4dlbpD55dTD0cFBA=
k8s.io/client-go v0.0.0-20240228230157-d99a76c51e14 h1:Pd0riBkFLU/4O4e9w5BP5mWjZpAezQP3nes1p5Fq+do=
k8s.io/client-go v0.0.0-20240228230157-d99a76c51e14/go.mod h1:POy8aYfSP1JEg4zV2/EFn50npEHhxwCnakxGKOLgBds=
k8s.io/component-base v0.0.0-20240227002902-6c2a49d37aa4 h1:QaNZj1n3+yyPNYz682bWlcjdJjAuSqSsvO5fDGQafxY=
k8s.io/component-base v0.0.0-20240227002902-6c2a49d37aa4/go.mod h1:Bf74PikueyXc4hduq9Fk6HNHav/OCym3S0shcPoh/mA=
k8s.io/component-helpers v0.0.0-20240221203355-866cab6f8733 h1:yjFDI+d1a3OGh7dOUZqP6vnUst/MFweCakqhPH0hILc=
k8s.io/component-helpers v0.0.0-20240221203355-866cab6f8733/go.mod h1:6EIc878uhjskixew9YOcZJFNzokUEKe0JfdWw4Gi6CM=
k8s.io/controller-manager v0.0.0-20240228234348-3c1067cedea3 h1:6WU4fLy75Y//fnBpA/IQSTGW6yMc3Fo6g4s0BgLAl+k=
k8s.io/controller-manager v0.0.0-20240228234348-3c1067cedea3/go.mod h1:FEyeEtbHduFzU2AQRaTlw4m9GBLsHUJRQmVOzl4MypQ=
k8s.io/controller-manager v0.0.0-20240229125230-05eba06abdf4 h1:wzPr8G2NIYN+UeTtE0m+nnxeyXoaWzZ1GJJzo3k3r7I=
k8s.io/controller-manager v0.0.0-20240229125230-05eba06abdf4/go.mod h1:RxcOsgmhlIl8AZ3r78oXCEVH9xBl3YzqmV7dXsC4FzY=
k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw=
k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kms v0.0.0-20240221203502-15393f39a6fb h1:i1haEXXv7oYb1tD43RCS7Yww0JAM63QIyMw9HZeRmDw=
Expand Down

0 comments on commit 123fab8

Please sign in to comment.