simplify openshift controller manager startup

Author: deads2k

Diff for: pkg/cmd/server/origin/controller/config.go

@@ -1,20 +1,15 @@
 package controller
 
 import (
-	"fmt"
-	"io/ioutil"
 	"path"
 	"time"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/util/cert"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
-	kcontroller "k8s.io/kubernetes/pkg/controller"
 	serviceaccountadmission "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
 
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
-	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	"github.com/openshift/origin/pkg/cmd/util/variable"
 )
 
@@ -56,8 +51,6 @@ func getOpenShiftClientEnvVars(options configapi.MasterConfig) ([]kapi.EnvVar, e
 // OpenshiftControllerConfig is the runtime (non-serializable) config object used to
 // launch the set of openshift (not kube) controllers.
 type OpenshiftControllerConfig struct {
-	ServiceAccountTokenControllerOptions ServiceAccountTokenControllerOptions
-
 	ServiceAccountControllerOptions ServiceAccountControllerOptions
 
 	BuildControllerConfig BuildControllerConfig
@@ -83,8 +76,6 @@ type OpenshiftControllerConfig struct {
 func (c *OpenshiftControllerConfig) GetControllerInitializers() (map[string]InitFunc, error) {
 	ret := map[string]InitFunc{}
 
-	ret["openshift.io/serviceaccount"] = c.ServiceAccountControllerOptions.RunController
-
 	ret["openshift.io/serviceaccount-pull-secrets"] = RunServiceAccountPullSecretsController
 	ret["openshift.io/origin-namespace"] = RunOriginNamespaceController
 	ret["openshift.io/service-serving-cert"] = c.ServiceServingCertsControllerOptions.RunController
@@ -115,63 +106,10 @@ func (c *OpenshiftControllerConfig) GetControllerInitializers() (map[string]Init
 	return ret, nil
 }
 
-// NewOpenShiftControllerPreStartInitializers returns list of initializers for controllers
-// that needed to be run before any other controller is started.
-// Typically this has to done for the serviceaccount-token controller as it provides
-// tokens to other controllers.
-func (c *OpenshiftControllerConfig) ServiceAccountContentControllerInit() InitFunc {
-	return c.ServiceAccountTokenControllerOptions.RunController
-}
-
 func BuildOpenshiftControllerConfig(options configapi.MasterConfig) (*OpenshiftControllerConfig, error) {
 	var err error
 	ret := &OpenshiftControllerConfig{}
 
-	_, loopbackClientConfig, err := configapi.GetInternalKubeClient(options.MasterClients.OpenShiftLoopbackKubeConfig, options.MasterClients.OpenShiftLoopbackClientConnectionOverrides)
-	if err != nil {
-		return nil, err
-	}
-
-	ret.ServiceAccountTokenControllerOptions = ServiceAccountTokenControllerOptions{
-		RootClientBuilder: kcontroller.SimpleControllerClientBuilder{
-			ClientConfig: loopbackClientConfig,
-		},
-	}
-	if len(options.ServiceAccountConfig.PrivateKeyFile) > 0 {
-		ret.ServiceAccountTokenControllerOptions.PrivateKey, err = cert.PrivateKeyFromFile(options.ServiceAccountConfig.PrivateKeyFile)
-		if err != nil {
-			return nil, fmt.Errorf("error reading signing key for Service Account Token Manager: %v", err)
-		}
-	}
-	if len(options.ServiceAccountConfig.MasterCA) > 0 {
-		ret.ServiceAccountTokenControllerOptions.RootCA, err = ioutil.ReadFile(options.ServiceAccountConfig.MasterCA)
-		if err != nil {
-			return nil, fmt.Errorf("error reading master ca file for Service Account Token Manager: %s: %v", options.ServiceAccountConfig.MasterCA, err)
-		}
-		if _, err := cert.ParseCertsPEM(ret.ServiceAccountTokenControllerOptions.RootCA); err != nil {
-			return nil, fmt.Errorf("error parsing master ca file for Service Account Token Manager: %s: %v", options.ServiceAccountConfig.MasterCA, err)
-		}
-	}
-	if options.ControllerConfig.ServiceServingCert.Signer != nil && len(options.ControllerConfig.ServiceServingCert.Signer.CertFile) > 0 {
-		certFile := options.ControllerConfig.ServiceServingCert.Signer.CertFile
-		serviceServingCA, err := ioutil.ReadFile(certFile)
-		if err != nil {
-			return nil, fmt.Errorf("error reading ca file for Service Serving Certificate Signer: %s: %v", certFile, err)
-		}
-		if _, err := crypto.CertsFromPEM(serviceServingCA); err != nil {
-			return nil, fmt.Errorf("error parsing ca file for Service Serving Certificate Signer: %s: %v", certFile, err)
-		}
-
-		// if we have a rootCA bundle add that too.  The rootCA will be used when hitting the default master service, since those are signed
-		// using a different CA by default.  The rootCA's key is more closely guarded than ours and if it is compromised, that power could
-		// be used to change the trusted signers for every pod anyway, so we're already effectively trusting it.
-		if len(ret.ServiceAccountTokenControllerOptions.RootCA) > 0 {
-			ret.ServiceAccountTokenControllerOptions.ServiceServingCA = append(ret.ServiceAccountTokenControllerOptions.ServiceServingCA, ret.ServiceAccountTokenControllerOptions.RootCA...)
-			ret.ServiceAccountTokenControllerOptions.ServiceServingCA = append(ret.ServiceAccountTokenControllerOptions.ServiceServingCA, []byte("\n")...)
-		}
-		ret.ServiceAccountTokenControllerOptions.ServiceServingCA = append(ret.ServiceAccountTokenControllerOptions.ServiceServingCA, serviceServingCA...)
-	}
-
 	ret.ServiceAccountControllerOptions = ServiceAccountControllerOptions{
 		ManagedNames: options.ServiceAccountConfig.ManagedNames,
 	}

Diff for: pkg/cmd/server/origin/controller/serviceaccount.go

@@ -4,9 +4,7 @@ import (
 	"github.com/golang/glog"
 
 	kapiv1 "k8s.io/api/core/v1"
-	"k8s.io/kubernetes/pkg/controller"
 	sacontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
-	"k8s.io/kubernetes/pkg/serviceaccount"
 
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 	serviceaccountcontrollers "github.com/openshift/origin/pkg/serviceaccounts/controllers"
@@ -49,37 +47,6 @@ func (c *ServiceAccountControllerOptions) RunController(ctx ControllerContext) (
 	return true, nil
 }
 
-type ServiceAccountTokenControllerOptions struct {
-	RootCA           []byte
-	ServiceServingCA []byte
-	PrivateKey       interface{}
-
-	RootClientBuilder controller.SimpleControllerClientBuilder
-}
-
-func (c *ServiceAccountTokenControllerOptions) RunController(ctx ControllerContext) (bool, error) {
-	if c.PrivateKey == nil {
-		glog.Infof("Skipped starting Service Account Token Manager, no private key specified")
-		return false, nil
-	}
-
-	controller, err := sacontroller.NewTokensController(
-		ctx.ExternalKubeInformers.Core().V1().ServiceAccounts(),
-		ctx.ExternalKubeInformers.Core().V1().Secrets(),
-		c.RootClientBuilder.ClientOrDie(bootstrappolicy.InfraServiceAccountTokensControllerServiceAccountName),
-		sacontroller.TokensControllerOptions{
-			TokenGenerator:   serviceaccount.JWTTokenGenerator(c.PrivateKey),
-			RootCA:           c.RootCA,
-			ServiceServingCA: c.ServiceServingCA,
-		},
-	)
-	if err != nil {
-		return true, nil
-	}
-	go controller.Run(int(ctx.OpenshiftControllerOptions.ServiceAccountTokenOptions.ConcurrentSyncs), ctx.Stop)
-	return true, nil
-}
-
 func RunServiceAccountPullSecretsController(ctx ControllerContext) (bool, error) {
 	kc := ctx.ClientBuilder.ClientOrDie(bootstrappolicy.InfraServiceAccountPullSecretsControllerServiceAccountName)
 

Diff for: pkg/cmd/server/start/start_kube_controller_manager.go

@@ -21,8 +21,6 @@ func computeKubeControllerManagerArgs(kubeconfigFile, saPrivateKeyFile, saRootCA
 			"-tokencleaner",
 			// we have to configure this separately until it is generic
 			"-horizontalpodautoscaling",
-			// we carry patches on this. For now....
-			"-serviceaccount-token",
 		}
 	}
 	if _, ok := cmdLineArgs["service-account-private-key-file"]; !ok {

Diff for: pkg/cmd/server/start/start_master.go

@@ -6,12 +6,10 @@ import (
 	"io"
 	"io/ioutil"
 	"net"
-	"net/http"
 	"os"
 	"path"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"github.com/coreos/go-systemd/daemon"
 	"github.com/golang/glog"
@@ -21,7 +19,6 @@ import (
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/apimachinery/pkg/util/wait"
 	utilwait "k8s.io/apimachinery/pkg/util/wait"
 	clientgoclientset "k8s.io/client-go/kubernetes"
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -602,40 +599,6 @@ func startControllers(options configapi.MasterConfig, allocationController origi
 		return err
 	}
 
-	// We need to start the serviceaccount-tokens controller first as it provides token
-	// generation for other controllers.
-	startSATokenController := openshiftControllerConfig.ServiceAccountContentControllerInit()
-	if enabled, err := startSATokenController(controllerContext); err != nil {
-		return fmt.Errorf("Error starting serviceaccount-token controller: %v", err)
-	} else if !enabled {
-		glog.Warningf("Skipping serviceaccount-token controller")
-	} else {
-		glog.Infof("Started serviceaccount-token controller")
-	}
-
-	// The service account controllers require informers in order to create service account tokens
-	// for other controllers, which means we need to start their informers (which use the privileged
-	// loopback client) before the other controllers will run.
-	controllerContext.ExternalKubeInformers.Start(controllerContext.Stop)
-
-	// right now we have controllers which are relying on the ability to make requests before the bootstrap policy is in place
-	// In 3.7, we will be fixed by the post start hook that prevents readiness unless policy is in place
-	// for 3.6, just make sure we don't proceed until the garbage collector can hit discovery
-	// wait for bootstrap permissions to be established.  This check isn't perfect, but it ensures that at least the controllers checking discovery can succeed
-	gcClientset := controllerContext.ClientBuilder.ClientOrDie("generic-garbage-collector")
-	err = wait.PollImmediate(500*time.Millisecond, 30*time.Second, func() (bool, error) {
-		result := gcClientset.Discovery().RESTClient().Get().AbsPath("/apis").Do()
-		var statusCode int
-		result.StatusCode(&statusCode)
-		if statusCode >= http.StatusOK && statusCode < http.StatusMultipleChoices {
-			return true, nil
-		}
-		return false, nil
-	})
-	if err != nil {
-		return err
-	}
-
 	//  the service account passed for the recyclable volume plugins needs to exist.  We want to do this via the init function, but its a kube init function
 	// for the rebase, create that service account here
 	// TODO make this a lot cleaner