Skip to content

Commit b8b819d

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #895 from FelipeYepez/l4-move-service-controller
Move over Service controller - handle CCM LoadBalancerClass
2 parents 7b5c2d8 + c54921a commit b8b819d

File tree

11 files changed

+1421
-1
lines changed

11 files changed

+1421
-1
lines changed

‎cmd/cloud-controller-manager/BUILD‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ go_library(
1919
name = "cloud-controller-manager_lib",
2020
srcs = [
2121
"gkenetworkparamsetcontroller.go",
22+
"gkeservicecontroller.go",
2223
"main.go",
2324
"nodeipamcontroller.go",
2425
],
@@ -29,12 +30,14 @@ go_library(
2930
"//pkg/controller/nodeipam",
3031
"//pkg/controller/nodeipam/config",
3132
"//pkg/controller/nodeipam/ipam",
33+
"//pkg/controller/service",
3234
"//providers/gce",
3335
"//vendor/github.com/GoogleCloudPlatform/gke-networking-api/client/network/clientset/versioned",
3436
"//vendor/github.com/GoogleCloudPlatform/gke-networking-api/client/network/informers/externalversions",
3537
"//vendor/github.com/GoogleCloudPlatform/gke-networking-api/client/nodetopology/clientset/versioned",
3638
"//vendor/github.com/spf13/pflag",
3739
"//vendor/k8s.io/apimachinery/pkg/util/wait",
40+
"//vendor/k8s.io/apiserver/pkg/util/feature",
3841
"//vendor/k8s.io/cloud-provider",
3942
"//vendor/k8s.io/cloud-provider/app",
4043
"//vendor/k8s.io/cloud-provider/app/config",
@@ -58,10 +61,15 @@ image(binary = ":cloud-controller-manager")
5861

5962
go_test(
6063
name = "cloud-controller-manager_test",
61-
srcs = ["nodeipamcontroller_test.go"],
64+
srcs = [
65+
"gkeservicecontroller_test.go",
66+
"nodeipamcontroller_test.go",
67+
],
6268
embed = [":cloud-controller-manager_lib"],
6369
deps = [
6470
"//pkg/controller/nodeipam/config",
71+
"//pkg/controller/service",
72+
"//vendor/k8s.io/api/core/v1:core",
6573
"//vendor/k8s.io/cloud-provider",
6674
"//vendor/k8s.io/cloud-provider/app/config",
6775
"//vendor/k8s.io/cloud-provider/config",

‎cmd/cloud-controller-manager/gkeservicecontroller.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package main
2+
3+
import (
4+
"context"
5+
6+
utilfeature "k8s.io/apiserver/pkg/util/feature"
7+
cloudprovider "k8s.io/cloud-provider"
8+
gkeservicecontroller "k8s.io/cloud-provider-gcp/pkg/controller/service"
9+
"k8s.io/cloud-provider/app"
10+
cloudcontrollerconfig "k8s.io/cloud-provider/app/config"
11+
controllermanagerapp "k8s.io/controller-manager/app"
12+
"k8s.io/controller-manager/controller"
13+
"k8s.io/klog/v2"
14+
)
15+
16+
// startGkeServiceControllerWrapper is used to take cloud config as input and start the GKE service controller
17+
func startGkeServiceControllerWrapper(initContext app.ControllerInitContext, completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) app.InitFunc {
18+
return func(ctx context.Context, controllerContext controllermanagerapp.ControllerContext) (controller.Interface, bool, error) {
19+
return startGkeServiceController(ctx, initContext, controllerContext, completedConfig, cloud)
20+
}
21+
}
22+
23+
func startGkeServiceController(ctx context.Context, initContext app.ControllerInitContext, controlexContext controllermanagerapp.ControllerContext, completedConfig *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface) (controller.Interface, bool, error) {
24+
// Start the service controller
25+
serviceController, err := gkeservicecontroller.New(
26+
cloud,
27+
completedConfig.ClientBuilder.ClientOrDie(initContext.ClientName),
28+
completedConfig.SharedInformers.Core().V1().Services(),
29+
completedConfig.SharedInformers.Core().V1().Nodes(),
30+
completedConfig.ComponentConfig.KubeCloudShared.ClusterName,
31+
utilfeature.DefaultFeatureGate,
32+
)
33+
if err != nil {
34+
// This error shouldn't fail. It lives like this as a legacy.
35+
klog.Errorf("Failed to start service controller: %v", err)
36+
return nil, false, nil
37+
}
38+
39+
go serviceController.Run(ctx, int(completedConfig.ComponentConfig.ServiceController.ConcurrentServiceSyncs), controlexContext.ControllerManagerMetrics)
40+
41+
return nil, true, nil
42+
}

‎cmd/cloud-controller-manager/gkeservicecontroller_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package main
2+
3+
import (
4+
"testing"
5+
6+
v1 "k8s.io/api/core/v1"
7+
gkeservicecontroller "k8s.io/cloud-provider-gcp/pkg/controller/service"
8+
)
9+
10+
// TestWantsLoadBalancer verifies that the forked and modified WantsLoadBalancer
11+
// function in k8s.io/cloud-provider-gcp/pkg/controller/service has the correct
12+
// logic. This test acts as a safeguard to prevent future updates from
13+
// overwriting the custom logic. It ensures that the controller correctly
14+
// processes services with GKE-specific legacy LoadBalancerClasses and,
15+
// just as importantly, ignores services that have no LoadBalancerClass set
16+
// or have a class that is not managed by this controller.
17+
func TestWantsLoadBalancer(t *testing.T) {
18+
gkeLegacyExternalClass := "networking.gke.io/l4-regional-external-legacy"
19+
gkeLegacyInternalClass := "networking.gke.io/l4-regional-internal-legacy"
20+
otherClass := "some-other-class"
21+
22+
testCases := []struct {
23+
name string
24+
service *v1.Service
25+
want bool
26+
}{
27+
{
28+
name: "service is not of type LoadBalancer",
29+
service: &v1.Service{
30+
Spec: v1.ServiceSpec{
31+
Type: v1.ServiceTypeClusterIP,
32+
},
33+
},
34+
want: false,
35+
},
36+
{
37+
name: "service is of type LoadBalancer with no loadBalancerClass",
38+
service: &v1.Service{
39+
Spec: v1.ServiceSpec{
40+
Type: v1.ServiceTypeLoadBalancer,
41+
},
42+
},
43+
want: false,
44+
},
45+
{
46+
name: "service is of type LoadBalancer with GKE legacy external loadBalancerClass",
47+
service: &v1.Service{
48+
Spec: v1.ServiceSpec{
49+
Type: v1.ServiceTypeLoadBalancer,
50+
LoadBalancerClass: &gkeLegacyExternalClass,
51+
},
52+
},
53+
want: true,
54+
},
55+
{
56+
name: "service is of type LoadBalancer with GKE legacy internal loadBalancerClass",
57+
service: &v1.Service{
58+
Spec: v1.ServiceSpec{
59+
Type: v1.ServiceTypeLoadBalancer,
60+
LoadBalancerClass: &gkeLegacyInternalClass,
61+
},
62+
},
63+
want: true,
64+
},
65+
{
66+
name: "service is of type LoadBalancer with other loadBalancerClass",
67+
service: &v1.Service{
68+
Spec: v1.ServiceSpec{
69+
Type: v1.ServiceTypeLoadBalancer,
70+
LoadBalancerClass: &otherClass,
71+
},
72+
},
73+
want: false,
74+
},
75+
}
76+
77+
for _, tc := range testCases {
78+
t.Run(tc.name, func(t *testing.T) {
79+
if got := gkeservicecontroller.WantsLoadBalancer(tc.service); got != tc.want {
80+
t.Errorf("WantsLoadBalancer() = %v, want %v", got, tc.want)
81+
}
82+
})
83+
}
84+
}

‎cmd/cloud-controller-manager/main.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ import (
4141
kcmnames "k8s.io/kubernetes/cmd/kube-controller-manager/names"
4242
)
4343

44+
const (
45+
gkeServiceLBControllerName = "gke-service-lb-controller"
46+
gkeServiceControllerClientName = "gke-service-controller"
47+
gkeServiceAlias = "gke-service"
48+
)
49+
4450
// enableMultiProject is bound to a command-line flag. When true, it enables the
4551
// projectFromNodeProviderID option of the GCE cloud provider, instructing it to
4652
// use the project specified in the Node's providerID for GCE API calls.
@@ -92,10 +98,19 @@ func main() {
9298
Constructor: startGkeNetworkParamSetControllerWrapper,
9399
}
94100

101+
controllerInitializers[gkeServiceLBControllerName] = app.ControllerInitFuncConstructor{
102+
InitContext: app.ControllerInitContext{
103+
ClientName: gkeServiceControllerClientName,
104+
},
105+
Constructor: startGkeServiceControllerWrapper,
106+
}
107+
95108
// add controllers disabled by default
96109
app.ControllersDisabledByDefault.Insert("gkenetworkparamset")
110+
app.ControllersDisabledByDefault.Insert(gkeServiceLBControllerName)
97111
aliasMap := names.CCMControllerAliases()
98112
aliasMap["nodeipam"] = kcmnames.NodeIpamController
113+
aliasMap[gkeServiceAlias] = gkeServiceLBControllerName
99114
command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, aliasMap, fss, wait.NeverStop)
100115

101116
logs.InitLogs()

‎pkg/controller/service/BUILD‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "service",
5+
srcs = [
6+
"controller.go",
7+
"controller_gke.go",
8+
"doc.go",
9+
"metrics.go",
10+
],
11+
importpath = "k8s.io/cloud-provider-gcp/pkg/controller/service",
12+
visibility = ["//visibility:public"],
13+
deps = [
14+
"//vendor/k8s.io/api/core/v1:core",
15+
"//vendor/k8s.io/apimachinery/pkg/api/errors",
16+
"//vendor/k8s.io/apimachinery/pkg/labels",
17+
"//vendor/k8s.io/apimachinery/pkg/util/runtime",
18+
"//vendor/k8s.io/apimachinery/pkg/util/sets",
19+
"//vendor/k8s.io/apimachinery/pkg/util/wait",
20+
"//vendor/k8s.io/client-go/informers/core/v1:core",
21+
"//vendor/k8s.io/client-go/kubernetes",
22+
"//vendor/k8s.io/client-go/kubernetes/scheme",
23+
"//vendor/k8s.io/client-go/kubernetes/typed/core/v1:core",
24+
"//vendor/k8s.io/client-go/listers/core/v1:core",
25+
"//vendor/k8s.io/client-go/tools/cache",
26+
"//vendor/k8s.io/client-go/tools/record",
27+
"//vendor/k8s.io/client-go/util/workqueue",
28+
"//vendor/k8s.io/cloud-provider",
29+
"//vendor/k8s.io/cloud-provider/api",
30+
"//vendor/k8s.io/cloud-provider/service/helpers",
31+
"//vendor/k8s.io/component-base/featuregate",
32+
"//vendor/k8s.io/component-base/metrics",
33+
"//vendor/k8s.io/component-base/metrics/legacyregistry",
34+
"//vendor/k8s.io/component-base/metrics/prometheus/controllers",
35+
"//vendor/k8s.io/klog/v2:klog",
36+
],
37+
)

‎pkg/controller/service/OWNERS‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
approvers:
2+
- aojea
3+
- bowei
4+
- mmamczur
5+
- thockin
6+
- felipeyepez
7+
labels:
8+
- sig/network

‎pkg/controller/service/README.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Service Controller
2+
3+
This service controller is a fork of the upstream service controller from `k8s.io/cloud-provider/controllers/service`.
4+
5+
## Modifications
6+
7+
The primary modification in this fork is to the `WantsLoadBalancer` function in `controller.go`. This change allows the controller to manage services that have a `loadBalancerClass` set to one of the following GKE-specific values:
8+
9+
- `networking.gke.io/l4-regional-external-legacy`
10+
- `networking.gke.io/l4-regional-internal-legacy`
11+
12+
The upstream controller ignores any service with a non-nil `loadBalancerClass`. This fork extends that logic to claim these specific classes for the GKE cloud provider. This ensures that this controller will only process services with a valid `loadBalancerClass`, while the default service controller handles all other services without a `LoadBalancerClass`.
13+
14+
When updating this forked controller from upstream, it is critical that the custom logic in the `WantsLoadBalancer` function is preserved. The purpose of this modification is to allow the controller to manage services that have a `loadBalancerClass` set to one of the GKE-specific values mentioned above.
15+
16+
## Controller Startup
17+
18+
This controller is started using a wrapper function, `startGkeServiceControllerWrapper`, located in `@cmd/cloud-controller-manager/gkeservicecontroller.go`. This wrapper initializes and runs the GKE-specific service controller when the `--controllers` flag includes `gke-service`.
19+
20+
## Configuration
21+
22+
The configuration for this controller, defined in `@vendor/k8s.io/cloud-provider/controllers/service/config/**`, is intentionally not forked. This approach allows the controller to reuse the existing command-line flags and configuration parameters from the upstream service controller, ensuring backward compatibility and simplifying maintenance.

0 commit comments

Comments
 (0)