refactor initialization of ingress-ip controller

openshift · Jun 14, 2017 · e4fd5bb · e4fd5bb
1 parent 3d90e8d
commit e4fd5bb
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 115 deletions.
diff --git a/pkg/cmd/server/bootstrappolicy/controller_policy.go b/pkg/cmd/server/bootstrappolicy/controller_policy.go
@@ -250,6 +250,16 @@ func init() {
 			eventsRule(),
 		},
 	})
+
+	// ingress-ip-controller
+	addControllerRole(rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraServiceIngressIPControllerServiceAccountName},
+		Rules: []rbac.PolicyRule{
+			rbac.NewRule("list", "watch", "update").Groups(kapiGroup).Resources("services").RuleOrDie(),
+			rbac.NewRule("update").Groups(kapiGroup).Resources("services/status").RuleOrDie(),
+			eventsRule(),
+		},
+	})
 }
 
 // ControllerRoles returns the cluster roles used by controllers

diff --git a/pkg/cmd/server/bootstrappolicy/infra_sa_policy.go b/pkg/cmd/server/bootstrappolicy/infra_sa_policy.go
@@ -38,6 +38,7 @@ const (
 	InfraResourceQuotaControllerServiceAccountName              = "resource-quota-controller"
 	InfraClusterQuotaReconciliationControllerServiceAccountName = "cluster-quota-reconciliation-controller"
 	InfraUnidlingControllerServiceAccountName                   = "unidling-controller"
+	InfraServiceIngressIPControllerServiceAccountName           = "service-ingress-ip-controller"
 
 	InfraPersistentVolumeBinderControllerServiceAccountName = "pv-binder-controller"
 	PersistentVolumeBinderControllerRoleName                = "system:pv-binder-controller"
@@ -54,8 +55,7 @@ const (
 	InfraServiceLoadBalancerControllerServiceAccountName = "service-load-balancer-controller"
 	ServiceLoadBalancerControllerRoleName                = "system:service-load-balancer-controller"
 
-	InfraServiceIngressIPControllerServiceAccountName = "service-ingress-ip-controller"
-	ServiceIngressIPControllerRoleName                = "system:service-ingress-ip-controller"
+	ServiceIngressIPControllerRoleName = "system:service-ingress-ip-controller"
 
 	InfraNodeBootstrapServiceAccountName = "node-bootstrapper"
 	NodeBootstrapRoleName                = "system:node-bootstrapper"
@@ -426,43 +426,6 @@ func init() {
 		panic(err)
 	}
 
-	err = InfraSAs.addServiceAccount(
-		InfraServiceIngressIPControllerServiceAccountName,
-		authorizationapi.ClusterRole{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: ServiceIngressIPControllerRoleName,
-			},
-			Rules: []authorizationapi.PolicyRule{
-				// Listing and watching services
-				{
-					APIGroups: []string{kapi.GroupName},
-					Verbs:     sets.NewString("list", "watch"),
-					Resources: sets.NewString("services"),
-				},
-				// IngressIPController.persistSpec changes the spec of the service
-				{
-					APIGroups: []string{kapi.GroupName},
-					Verbs:     sets.NewString("update"),
-					Resources: sets.NewString("services"),
-				},
-				// IngressIPController.persistStatus changes the status of the service
-				{
-					APIGroups: []string{kapi.GroupName},
-					Verbs:     sets.NewString("update"),
-					Resources: sets.NewString("services/status"),
-				},
-				// IngressIPController.recorder
-				{
-					Verbs:     sets.NewString("create", "update", "patch"),
-					Resources: sets.NewString("events"),
-				},
-			},
-		},
-	)
-	if err != nil {
-		panic(err)
-	}
-
 	err = InfraSAs.addServiceAccount(
 		InfraNodeBootstrapServiceAccountName,
 		authorizationapi.ClusterRole{

diff --git a/pkg/cmd/server/origin/controller.go b/pkg/cmd/server/origin/controller.go
@@ -172,5 +172,10 @@ func (c *MasterConfig) NewOpenshiftControllerInitializers() (map[string]controll
 	}
 	ret["openshift.io/unidling"] = unidlingController.RunController
 
+	ingressIPController := controller.IngressIPControllerConfig{
+		IngressIPSyncPeriod:  10 * time.Minute,
+		IngressIPNetworkCIDR: c.Options.NetworkConfig.IngressIPNetworkCIDR,
+	}
+	ret["openshift.io/ingress-ip"] = ingressIPController.RunController
 	return ret, nil
 }
diff --git a/pkg/cmd/server/origin/controller/network.go b/pkg/cmd/server/origin/controller/network.go
@@ -1,10 +1,15 @@
 package controller
 
 import (
+	"fmt"
+	"net"
+	"time"
+
 	osclient "github.com/openshift/origin/pkg/client"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 	sdnplugin "github.com/openshift/origin/pkg/sdn/plugin"
+	"github.com/openshift/origin/pkg/service/controller/ingressip"
 )
 
 type SDNControllerConfig struct {
@@ -29,3 +34,35 @@ func (c *SDNControllerConfig) RunController(ctx ControllerContext) (bool, error)
 	)
 	return true, err
 }
+
+type IngressIPControllerConfig struct {
+	IngressIPNetworkCIDR string
+	IngressIPSyncPeriod  time.Duration
+}
+
+func (c *IngressIPControllerConfig) RunController(ctx ControllerContext) (bool, error) {
+	if len(c.IngressIPNetworkCIDR) == 0 {
+		return true, nil
+	}
+
+	_, ipNet, err := net.ParseCIDR(c.IngressIPNetworkCIDR)
+	if err != nil {
+		return false, fmt.Errorf("unable to start ingress IP controller: %v", err)
+	}
+
+	if ipNet.IP.IsUnspecified() {
+		// TODO: Is this an error?
+		return true, nil
+	}
+
+	ingressIPController := ingressip.NewIngressIPController(
+		ctx.DeprecatedOpenshiftInformers.InternalKubernetesInformers().Core().InternalVersion().Services().Informer(),
+		ctx.ClientBuilder.KubeInternalClientOrDie(bootstrappolicy.InfraServiceIngressIPControllerServiceAccountName),
+		ctx.ClientBuilder.ClientOrDie(bootstrappolicy.InfraServiceIngressIPControllerServiceAccountName),
+		ipNet,
+		c.IngressIPSyncPeriod,
+	)
+	go ingressIPController.Run(ctx.Stop)
+
+	return true, nil
+}
diff --git a/pkg/cmd/server/origin/run_components.go b/pkg/cmd/server/origin/run_components.go
@@ -9,8 +9,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilwait "k8s.io/apimachinery/pkg/util/wait"
 	kapi "k8s.io/kubernetes/pkg/api"
-	kclientsetexternal "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
-	kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/registry/core/service/allocator"
 	etcdallocator "k8s.io/kubernetes/pkg/registry/core/service/allocator/storage"
 
@@ -21,11 +19,6 @@ import (
 	"github.com/openshift/origin/pkg/security/mcs"
 	"github.com/openshift/origin/pkg/security/uid"
 	"github.com/openshift/origin/pkg/security/uidallocator"
-	"github.com/openshift/origin/pkg/service/controller/ingressip"
-)
-
-const (
-	defaultIngressIPSyncPeriod time.Duration = 10 * time.Minute
 )
 
 // RunProjectAuthorizationCache starts the project authorization cache
@@ -157,30 +150,6 @@ func (c *MasterConfig) RunGroupCache() {
 	c.GroupCache.Run()
 }
 
-// RunIngressIPController starts the ingress ip controller if IngressIPNetworkCIDR is configured.
-func (c *MasterConfig) RunIngressIPController(internalKubeClientset kclientsetinternal.Interface, externalKubeClientset kclientsetexternal.Interface) {
-	if len(c.Options.NetworkConfig.IngressIPNetworkCIDR) == 0 {
-		return
-	}
-
-	_, ipNet, err := net.ParseCIDR(c.Options.NetworkConfig.IngressIPNetworkCIDR)
-	if err != nil {
-		// should have been caught with validation
-		glog.Fatalf("Unable to start ingress ip controller: %v", err)
-	}
-	if ipNet.IP.IsUnspecified() {
-		return
-	}
-	ingressIPController := ingressip.NewIngressIPController(
-		c.Informers.InternalKubernetesInformers().Core().InternalVersion().Services().Informer(),
-		internalKubeClientset,
-		externalKubeClientset,
-		ipNet,
-		defaultIngressIPSyncPeriod,
-	)
-	go ingressIPController.Run(utilwait.NeverStop)
-}
-
 func (c *MasterConfig) RunOriginToRBACSyncControllers() {
 	clusterRoles := authorizationsync.NewOriginToRBACClusterRoleController(
 		c.Informers.InternalKubernetesInformers().Rbac().InternalVersion().ClusterRoles(),

diff --git a/pkg/cmd/server/start/start_master.go b/pkg/cmd/server/start/start_master.go
@@ -801,6 +801,7 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
 		"openshift.io/cluster-quota-reconciliation",
 		"openshift.io/cluster-quota-mapping",
 		"openshift.io/unidling",
+		"openshift.io/ingress-ip",
 	)
 
 	if configapi.IsBuildEnabled(&oc.Options) {
@@ -838,12 +839,6 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
 
 	oc.RunOriginToRBACSyncControllers()
 
-	_, _, ingressIPClientInternal, ingressIPClientExternal, err := oc.GetServiceAccountClients(bootstrappolicy.InfraServiceIngressIPControllerServiceAccountName)
-	if err != nil {
-		glog.Fatalf("Could not get client: %v", err)
-	}
-	oc.RunIngressIPController(ingressIPClientInternal, ingressIPClientExternal)
-
 	glog.Infof("Started Origin Controllers")
 
 	return nil

diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml
@@ -890,6 +890,20 @@ items:
     namespace: openshift-infra
   userNames:
   - system:serviceaccount:openshift-infra:unidling-controller
+- apiVersion: v1
+  groupNames: null
+  kind: ClusterRoleBinding
+  metadata:
+    creationTimestamp: null
+    name: system:openshift:controller:service-ingress-ip-controller
+  roleRef:
+    name: system:openshift:controller:service-ingress-ip-controller
+  subjects:
+  - kind: ServiceAccount
+    name: service-ingress-ip-controller
+    namespace: openshift-infra
+  userNames:
+  - system:serviceaccount:openshift-infra:service-ingress-ip-controller
 - apiVersion: v1
   groupNames:
   - system:masters

diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml
@@ -3240,45 +3240,6 @@ items:
     - create
     - patch
     - update
-- apiVersion: v1
-  kind: ClusterRole
-  metadata:
-    annotations:
-      authorization.openshift.io/system-only: "true"
-    creationTimestamp: null
-    name: system:service-ingress-ip-controller
-  rules:
-  - apiGroups:
-    - ""
-    attributeRestrictions: null
-    resources:
-    - services
-    verbs:
-    - list
-    - watch
-  - apiGroups:
-    - ""
-    attributeRestrictions: null
-    resources:
-    - services
-    verbs:
-    - update
-  - apiGroups:
-    - ""
-    attributeRestrictions: null
-    resources:
-    - services/status
-    verbs:
-    - update
-  - apiGroups:
-    - ""
-    attributeRestrictions: null
-    resources:
-    - events
-    verbs:
-    - create
-    - patch
-    - update
 - apiVersion: v1
   kind: ClusterRole
   metadata:
@@ -4202,6 +4163,39 @@ items:
     - create
     - patch
     - update
+- apiVersion: v1
+  kind: ClusterRole
+  metadata:
+    annotations:
+      authorization.openshift.io/system-only: "true"
+    creationTimestamp: null
+    name: system:openshift:controller:service-ingress-ip-controller
+  rules:
+  - apiGroups:
+    - ""
+    attributeRestrictions: null
+    resources:
+    - services
+    verbs:
+    - list
+    - update
+    - watch
+  - apiGroups:
+    - ""
+    attributeRestrictions: null
+    resources:
+    - services/status
+    verbs:
+    - update
+  - apiGroups:
+    - ""
+    attributeRestrictions: null
+    resources:
+    - events
+    verbs:
+    - create
+    - patch
+    - update
 - apiVersion: v1
   kind: ClusterRole
   metadata: