Merge pull request #605 from kmala/master

ignore invalid providerid for cache indexer
kubernetes · Jun 9, 2023 · 676186d · 676186d
2 parents 9ffe0cd + 5d15f06
commit 676186d
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 5 deletions.
diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go
@@ -45,10 +45,8 @@ import (
 	"github.com/aws/aws-sdk-go/service/kms"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"gopkg.in/gcfg.v1"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/klog/v2"
-	netutils "k8s.io/utils/net"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -68,6 +66,8 @@ import (
 	cloudvolume "k8s.io/cloud-provider/volume"
 	volerr "k8s.io/cloud-provider/volume/errors"
 	volumehelpers "k8s.io/cloud-provider/volume/helpers"
+	"k8s.io/klog/v2"
+	netutils "k8s.io/utils/net"
 )
 
 // NLBHealthCheckRuleDescription is the comment used on a security group rule to
@@ -850,7 +850,9 @@ func InstanceIDIndexFunc(obj interface{}) ([]string, error) {
 	}
 	instanceID, err := KubernetesInstanceID(node.Spec.ProviderID).MapToAWSInstanceID()
 	if err != nil {
-		return []string{""}, fmt.Errorf("error mapping node %q's provider ID %q to instance ID: %v", node.Name, node.Spec.ProviderID, err)
+		//logging the error as warning as Informer.AddIndexers would panic if there is an error
+		klog.Warningf("error mapping node %q's provider ID %q to instance ID: %v", node.Name, node.Spec.ProviderID, err)
+		return []string{""}, nil
 	}
 	return []string{string(instanceID)}, nil
 }

diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go
@@ -36,7 +36,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
-
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -3980,3 +3979,58 @@ func TestDescribeInstances(t *testing.T) {
 		})
 	}
 }
+
+func TestInstanceIDIndexFunc(t *testing.T) {
+	type args struct {
+		obj interface{}
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []string
+		wantErr bool
+	}{
+		{
+			name: "returns empty on invalid provider id",
+			args: args{
+				obj: &v1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-node",
+					},
+					Spec: v1.NodeSpec{
+						ProviderID: "foo://com-2351",
+					},
+				},
+			},
+			want:    []string{""},
+			wantErr: false,
+		},
+		{
+			name: "returns correct instance id on valid provider id",
+			args: args{
+				obj: &v1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-valid-node",
+					},
+					Spec: v1.NodeSpec{
+						ProviderID: "aws:////i-12345678abcdef01",
+					},
+				},
+			},
+			want:    []string{"i-12345678abcdef01"},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := InstanceIDIndexFunc(tt.args.obj)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("InstanceIDIndexFunc() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("InstanceIDIndexFunc() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}