Merge pull request #973 from 08volt/release-1.35

[Cherry-Pick #972 and  #979] update EnsureLoadBalancer to return nil instead of status when syncResult is nil

Author: k8s-ci-robot

WORKSPACE

@@ -50,8 +50,11 @@ http_archive(
 
 http_archive(
     name = "io_bazel_rules_docker",
-    sha256 = "b1e80761a8a8243d03ebca8845e9cc1ba6c82ce7c5179ce2b295cd36f7e394bf",
-    urls = ["https://github.com/bazelbuild/rules_docker/releases/download/v0.25.0/rules_docker-v0.25.0.tar.gz"],
+    sha256 = "f6dcb97e992f13bc9effd794e9bb300f06b0dadc88061f81ae68d8d5994be964",
+    urls = [
+        "https://mirror.bazel.build/github.com/bazelbuild/rules_docker/releases/download/v0.26.0/rules_docker-v0.26.0.tar.gz",
+        "https://github.com/bazelbuild/rules_docker/releases/download/v0.26.0/rules_docker-v0.26.0.tar.gz",
+    ],
 )
 
 load("@bazel_skylib//lib:versions.bzl", "versions")

providers/gce/gce_loadbalancer.go

@@ -227,17 +227,25 @@ func (g *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, svc
 	}
 	if err != nil {
 		klog.Errorf("Failed to EnsureLoadBalancer(%s, %s, %s, %s, %s), err: %v", clusterName, svc.Namespace, svc.Name, loadBalancerName, g.region, err)
-		return syncResult.status, err
+		return nil, err
+	}
+
+	var status *v1.LoadBalancerStatus
+	var annotations map[string]string
+
+	if syncResult != nil {
+		status = syncResult.status
+		annotations = syncResult.annotations
 	}
 
 	if g.enableL4LBAnnotations {
-		if err = g.updateL4ResourcesAnnotations(ctx, svc, syncResult.annotations); err != nil {
-			return syncResult.status, fmt.Errorf("failed to set resource annotations, err: %w", err)
+		if err = g.updateL4ResourcesAnnotations(ctx, svc, annotations); err != nil {
+			return status, fmt.Errorf("failed to set resource annotations, err: %w", err)
 		}
 	}
 
 	klog.V(4).Infof("EnsureLoadBalancer(%s, %s, %s, %s, %s): done ensuring loadbalancer.", clusterName, svc.Namespace, svc.Name, loadBalancerName, g.region)
-	return syncResult.status, err
+	return status, err
 }
 
 func (g *Cloud) updateL4ResourcesAnnotations(ctx context.Context, svc *v1.Service, newL4LBAnnotations map[string]string) error {

providers/gce/gce_loadbalancer_test.go

@@ -29,8 +29,39 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	cloudprovider "k8s.io/cloud-provider"
+
+	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
+	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
+	"google.golang.org/api/compute/v1"
 )
 
+func TestEnsureLoadBalancerError(t *testing.T) {
+	t.Parallel()
+
+	vals := DefaultTestClusterValues()
+	gce, err := fakeGCECloud(vals)
+	require.NoError(t, err)
+
+	nodeNames := []string{"test-node-1"}
+	nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
+	require.NoError(t, err)
+
+	apiService := fakeLoadbalancerService(string(LBTypeInternal))
+	apiService, err = gce.client.CoreV1().Services(apiService.Namespace).Create(context.TODO(), apiService, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	// Inject error
+	mockGCE := gce.c.(*cloud.MockGCE)
+	mockGCE.MockInstanceGroups.GetHook = func(ctx context.Context, key *meta.Key, m *cloud.MockInstanceGroups, options ...cloud.Option) (bool, *compute.InstanceGroup, error) {
+		return true, nil, fmt.Errorf("injected error")
+	}
+
+	status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes)
+	require.Error(t, err)
+	assert.Nil(t, status)
+	assert.Contains(t, err.Error(), "injected error")
+}
+
 func TestGetLoadBalancer(t *testing.T) {
 	t.Parallel()