Skip to content

Commit 5a18b2a

Browse files
Merge pull request #16510 from deads2k/controller-13-simplify
Automatic merge from submit-queue (batch tested with PRs 16534, 16533, 16510, 16508, 16392) simplify controller start Last commit is unique. This removes the direct usage of the controller manager options struct for unrelated controller initialization. We still use it for parsing (I didn't want to get stuck writing new config. This isn't net new usage. I just made the links obvious).
2 parents 9a9d413 + c3194de commit 5a18b2a

File tree

9 files changed

+143
-144
lines changed

9 files changed

+143
-144
lines changed

pkg/cmd/server/kubernetes/master/master_config.go

-77
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/golang/glog"
2020
"gopkg.in/natefinch/lumberjack.v2"
2121

22-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2322
openapicommon "k8s.io/apimachinery/pkg/openapi"
2423
"k8s.io/apimachinery/pkg/runtime"
2524
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -48,16 +47,13 @@ import (
4847
auditlog "k8s.io/apiserver/plugin/pkg/audit/log"
4948
auditwebhook "k8s.io/apiserver/plugin/pkg/audit/webhook"
5049
kapiserveroptions "k8s.io/kubernetes/cmd/kube-apiserver/app/options"
51-
cmapp "k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
5250
kapi "k8s.io/kubernetes/pkg/api"
5351
"k8s.io/kubernetes/pkg/apis/apps"
5452
"k8s.io/kubernetes/pkg/apis/autoscaling"
5553
"k8s.io/kubernetes/pkg/apis/batch"
5654
batchv2alpha1 "k8s.io/kubernetes/pkg/apis/batch/v2alpha1"
57-
"k8s.io/kubernetes/pkg/apis/componentconfig"
5855
"k8s.io/kubernetes/pkg/apis/extensions"
5956
"k8s.io/kubernetes/pkg/apis/networking"
60-
"k8s.io/kubernetes/pkg/cloudprovider"
6157
"k8s.io/kubernetes/pkg/master"
6258
"k8s.io/kubernetes/pkg/registry/cachesize"
6359
"k8s.io/kubernetes/pkg/registry/core/endpoint"
@@ -71,7 +67,6 @@ import (
7167
"github.com/openshift/origin/pkg/cmd/flagtypes"
7268
configapi "github.com/openshift/origin/pkg/cmd/server/api"
7369
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
74-
"github.com/openshift/origin/pkg/cmd/server/cm"
7570
"github.com/openshift/origin/pkg/cmd/server/crypto"
7671
"github.com/openshift/origin/pkg/cmd/server/election"
7772
cmdutil "github.com/openshift/origin/pkg/cmd/util"
@@ -326,78 +321,6 @@ func buildUpstreamClientCARegistrationHook(s *kapiserveroptions.ServerRunOptions
326321
}, nil
327322
}
328323

329-
func BuildControllerManagerServer(masterConfig configapi.MasterConfig) (*cmapp.CMServer, cloudprovider.Interface, error) {
330-
podEvictionTimeout, err := time.ParseDuration(masterConfig.KubernetesMasterConfig.PodEvictionTimeout)
331-
if err != nil {
332-
return nil, nil, fmt.Errorf("unable to parse PodEvictionTimeout: %v", err)
333-
}
334-
335-
// Defaults are tested in TestCMServerDefaults
336-
cmserver := cmapp.NewCMServer()
337-
// Adjust defaults
338-
cmserver.ClusterSigningCertFile = ""
339-
cmserver.ClusterSigningKeyFile = ""
340-
cmserver.LeaderElection.RetryPeriod = metav1.Duration{Duration: 3 * time.Second}
341-
cmserver.ClusterSigningDuration = metav1.Duration{Duration: 0}
342-
cmserver.Address = "" // no healthz endpoint
343-
cmserver.Port = 0 // no healthz endpoint
344-
cmserver.EnableGarbageCollector = true
345-
cmserver.PodEvictionTimeout = metav1.Duration{Duration: podEvictionTimeout}
346-
cmserver.VolumeConfiguration.EnableDynamicProvisioning = masterConfig.VolumeConfig.DynamicProvisioningEnabled
347-
348-
// IF YOU ADD ANYTHING TO THIS LIST, MAKE SURE THAT YOU UPDATE THEIR STRATEGIES TO PREVENT GC FINALIZERS
349-
cmserver.GCIgnoredResources = append(cmserver.GCIgnoredResources,
350-
// explicitly disabled from GC for now - not enough value to track them
351-
componentconfig.GroupResource{Group: "authorization.openshift.io", Resource: "rolebindingrestrictions"},
352-
componentconfig.GroupResource{Group: "network.openshift.io", Resource: "clusternetworks"},
353-
componentconfig.GroupResource{Group: "network.openshift.io", Resource: "egressnetworkpolicies"},
354-
componentconfig.GroupResource{Group: "network.openshift.io", Resource: "hostsubnets"},
355-
componentconfig.GroupResource{Group: "network.openshift.io", Resource: "netnamespaces"},
356-
componentconfig.GroupResource{Group: "oauth.openshift.io", Resource: "oauthclientauthorizations"},
357-
componentconfig.GroupResource{Group: "oauth.openshift.io", Resource: "oauthclients"},
358-
componentconfig.GroupResource{Group: "quota.openshift.io", Resource: "clusterresourcequotas"},
359-
componentconfig.GroupResource{Group: "user.openshift.io", Resource: "groups"},
360-
componentconfig.GroupResource{Group: "user.openshift.io", Resource: "identities"},
361-
componentconfig.GroupResource{Group: "user.openshift.io", Resource: "users"},
362-
componentconfig.GroupResource{Group: "image.openshift.io", Resource: "images"},
363-
364-
// virtual resource
365-
componentconfig.GroupResource{Group: "project.openshift.io", Resource: "projects"},
366-
// these resources contain security information in their names, and we don't need to track them
367-
componentconfig.GroupResource{Group: "oauth.openshift.io", Resource: "oauthaccesstokens"},
368-
componentconfig.GroupResource{Group: "oauth.openshift.io", Resource: "oauthauthorizetokens"},
369-
// exposed already as cronjobs
370-
componentconfig.GroupResource{Group: "batch", Resource: "scheduledjobs"},
371-
// exposed already as extensions v1beta1 by other controllers
372-
componentconfig.GroupResource{Group: "apps", Resource: "deployments"},
373-
// exposed as autoscaling v1
374-
componentconfig.GroupResource{Group: "extensions", Resource: "horizontalpodautoscalers"},
375-
)
376-
377-
// resolve extended arguments
378-
// TODO: this should be done in config validation (along with the above) so we can provide
379-
// proper errors
380-
if err := cmdflags.Resolve(masterConfig.KubernetesMasterConfig.ControllerArguments, cm.OriginControllerManagerAddFlags(cmserver)); len(err) > 0 {
381-
return nil, nil, kerrors.NewAggregate(err)
382-
}
383-
cloud, err := cloudprovider.InitCloudProvider(cmserver.CloudProvider, cmserver.CloudConfigFile)
384-
if err != nil {
385-
return nil, nil, err
386-
}
387-
if cloud != nil {
388-
if cloud.HasClusterID() == false {
389-
if cmserver.AllowUntaggedCloud == true {
390-
glog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues")
391-
} else {
392-
return nil, nil, fmt.Errorf("no ClusterID Found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option")
393-
}
394-
}
395-
glog.V(2).Infof("Successfully initialized cloud provider: %q from the config file: %q\n", cmserver.CloudProvider, cmserver.CloudConfigFile)
396-
}
397-
398-
return cmserver, cloud, nil
399-
}
400-
401324
func buildProxyClientCerts(masterConfig configapi.MasterConfig) ([]tls.Certificate, error) {
402325
var proxyClientCerts []tls.Certificate
403326
if len(masterConfig.KubernetesMasterConfig.ProxyClientInfo.CertFile) > 0 {

pkg/cmd/server/origin/controller/autoscaling.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ type HorizontalPodAutoscalerControllerConfig struct {
1919
}
2020

2121
func (c *HorizontalPodAutoscalerControllerConfig) RunController(originCtx ControllerContext) (bool, error) {
22-
ctx := originCtx.KubeControllerContext
23-
24-
hpaClientConfig := ctx.ClientBuilder.ConfigOrDie(bootstrappolicy.InfraHorizontalPodAutoscalerControllerServiceAccountName)
22+
hpaClientConfig, err := originCtx.ClientBuilder.Config(bootstrappolicy.InfraHorizontalPodAutoscalerControllerServiceAccountName)
23+
if err != nil {
24+
return true, err
25+
}
2526

2627
hpaClient, err := kubeclientset.NewForConfig(hpaClientConfig)
2728
if err != nil {
@@ -52,11 +53,11 @@ func (c *HorizontalPodAutoscalerControllerConfig) RunController(originCtx Contro
5253
delegatingScalesGetter,
5354
hpaClient.Autoscaling(),
5455
replicaCalc,
55-
ctx.InformerFactory.Autoscaling().V1().HorizontalPodAutoscalers(),
56-
ctx.Options.HorizontalPodAutoscalerSyncPeriod.Duration,
57-
ctx.Options.HorizontalPodAutoscalerUpscaleForbiddenWindow.Duration,
58-
ctx.Options.HorizontalPodAutoscalerDownscaleForbiddenWindow.Duration,
59-
).Run(ctx.Stop)
56+
originCtx.ExternalKubeInformers.Autoscaling().V1().HorizontalPodAutoscalers(),
57+
originCtx.OpenshiftControllerOptions.HPAControllerOptions.SyncPeriod.Duration,
58+
originCtx.OpenshiftControllerOptions.HPAControllerOptions.UpscaleForbiddenWindow.Duration,
59+
originCtx.OpenshiftControllerOptions.HPAControllerOptions.DownscaleForbiddenWindow.Duration,
60+
).Run(originCtx.Stop)
6061

6162
return true, nil
6263
}

pkg/cmd/server/origin/controller/interfaces.go

+26-9
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package controller
33
import (
44
"github.com/golang/glog"
55

6-
kubecontroller "k8s.io/kubernetes/cmd/kube-controller-manager/app"
6+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
77
kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
88
kexternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions"
99
kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
@@ -25,7 +25,7 @@ import (
2525
)
2626

2727
type ControllerContext struct {
28-
KubeControllerContext kubecontroller.ControllerContext
28+
OpenshiftControllerOptions OpenshiftControllerOptions
2929

3030
// ClientBuilder will provide a client for this controller to use
3131
ClientBuilder ControllerClientBuilder
@@ -44,6 +44,30 @@ type ControllerContext struct {
4444
Stop <-chan struct{}
4545
}
4646

47+
// OpenshiftControllerOptions contain the options used to run the controllers. Eventually we need to construct a way to properly
48+
// configure these in a config struct. This at least lets us know what we have.
49+
type OpenshiftControllerOptions struct {
50+
HPAControllerOptions HPAControllerOptions
51+
ResourceQuotaOptions ResourceQuotaOptions
52+
ServiceAccountTokenOptions ServiceAccountTokenOptions
53+
}
54+
55+
type HPAControllerOptions struct {
56+
SyncPeriod metav1.Duration
57+
UpscaleForbiddenWindow metav1.Duration
58+
DownscaleForbiddenWindow metav1.Duration
59+
}
60+
61+
type ResourceQuotaOptions struct {
62+
ConcurrentSyncs int32
63+
SyncPeriod metav1.Duration
64+
MinResyncPeriod metav1.Duration
65+
}
66+
67+
type ServiceAccountTokenOptions struct {
68+
ConcurrentSyncs int32
69+
}
70+
4771
// TODO wire this up to something that handles the names. The logic is available upstream, we just have to wire to it
4872
func (c ControllerContext) IsControllerEnabled(name string) bool {
4973
return true
@@ -227,10 +251,3 @@ func (b OpenshiftControllerClientBuilder) OpenshiftInternalNetworkClientOrDie(na
227251
}
228252
return client
229253
}
230-
231-
// FromKubeInitFunc adapts a kube init func to an openshift one
232-
func FromKubeInitFunc(initFn kubecontroller.InitFunc) InitFunc {
233-
return func(ctx ControllerContext) (bool, error) {
234-
return initFn(ctx.KubeControllerContext)
235-
}
236-
}

pkg/cmd/server/origin/controller/quota.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ import (
1515
)
1616

1717
func RunResourceQuotaManager(ctx ControllerContext) (bool, error) {
18-
concurrentResourceQuotaSyncs := int(ctx.KubeControllerContext.Options.ConcurrentResourceQuotaSyncs)
19-
resourceQuotaSyncPeriod := ctx.KubeControllerContext.Options.ResourceQuotaSyncPeriod.Duration
20-
replenishmentSyncPeriodFunc := calculateResyncPeriod(ctx.KubeControllerContext.Options.MinResyncPeriod.Duration)
18+
concurrentResourceQuotaSyncs := int(ctx.OpenshiftControllerOptions.ResourceQuotaOptions.ConcurrentSyncs)
19+
resourceQuotaSyncPeriod := ctx.OpenshiftControllerOptions.ResourceQuotaOptions.SyncPeriod.Duration
20+
replenishmentSyncPeriodFunc := calculateResyncPeriod(ctx.OpenshiftControllerOptions.ResourceQuotaOptions.MinResyncPeriod.Duration)
2121
saName := "resourcequota-controller"
2222

2323
resourceQuotaRegistry := quota.NewOriginQuotaRegistry(

pkg/cmd/server/origin/controller/serviceaccount.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (c *ServiceAccountTokenControllerOptions) RunController(ctx ControllerConte
6868
RootCA: c.RootCA,
6969
ServiceServingCA: c.ServiceServingCA,
7070
},
71-
).Run(int(ctx.KubeControllerContext.Options.ConcurrentSATokenSyncs), ctx.Stop)
71+
).Run(int(ctx.OpenshiftControllerOptions.ServiceAccountTokenOptions.ConcurrentSyncs), ctx.Stop)
7272
return true, nil
7373
}
7474

pkg/cmd/server/start/controllers.go

+92-36
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,50 @@
11
package start
22

33
import (
4+
"fmt"
5+
"net/http"
6+
"time"
7+
8+
"github.com/golang/glog"
9+
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
kutilerrors "k8s.io/apimachinery/pkg/util/errors"
12+
"k8s.io/apimachinery/pkg/util/wait"
413
"k8s.io/client-go/rest"
5-
cmapp "k8s.io/kubernetes/cmd/kube-controller-manager/app"
614
cmappoptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
7-
"k8s.io/kubernetes/pkg/cloudprovider"
15+
"k8s.io/kubernetes/pkg/apis/componentconfig"
16+
kclientsetexternal "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
817
"k8s.io/kubernetes/pkg/controller"
918

10-
configapi "github.com/openshift/origin/pkg/cmd/server/api"
1119
"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
20+
"github.com/openshift/origin/pkg/cmd/server/cm"
1221
origincontrollers "github.com/openshift/origin/pkg/cmd/server/origin/controller"
22+
cmdflags "github.com/openshift/origin/pkg/cmd/util/flags"
1323
)
1424

15-
func getControllerContext(options configapi.MasterConfig, controllerManagerOptions *cmappoptions.CMServer, cloudProvider cloudprovider.Interface, informers *informers, stopCh <-chan struct{}) (origincontrollers.ControllerContext, error) {
16-
loopbackConfig, _, kubeExternal, _, err := getAllClients(options)
17-
if err != nil {
18-
return origincontrollers.ControllerContext{}, err
19-
}
25+
func newControllerContext(
26+
openshiftControllerOptions origincontrollers.OpenshiftControllerOptions,
27+
privilegedLoopbackConfig *rest.Config,
28+
kubeExternal kclientsetexternal.Interface,
29+
informers *informers,
30+
stopCh <-chan struct{},
31+
) origincontrollers.ControllerContext {
32+
2033
// divide up the QPS since it re-used separately for every client
2134
// TODO, eventually make this configurable individually in some way.
22-
if loopbackConfig.QPS > 0 {
23-
loopbackConfig.QPS = loopbackConfig.QPS/10 + 1
35+
if privilegedLoopbackConfig.QPS > 0 {
36+
privilegedLoopbackConfig.QPS = privilegedLoopbackConfig.QPS/10 + 1
2437
}
25-
if loopbackConfig.Burst > 0 {
26-
loopbackConfig.Burst = loopbackConfig.Burst/10 + 1
27-
}
28-
29-
rootClientBuilder := controller.SimpleControllerClientBuilder{
30-
ClientConfig: loopbackConfig,
31-
}
32-
33-
availableResources, err := cmapp.GetAvailableResources(rootClientBuilder)
34-
if err != nil {
35-
return origincontrollers.ControllerContext{}, err
38+
if privilegedLoopbackConfig.Burst > 0 {
39+
privilegedLoopbackConfig.Burst = privilegedLoopbackConfig.Burst/10 + 1
3640
}
3741

3842
openshiftControllerContext := origincontrollers.ControllerContext{
39-
KubeControllerContext: cmapp.ControllerContext{
40-
ClientBuilder: controller.SAControllerClientBuilder{
41-
ClientConfig: rest.AnonymousClientConfig(loopbackConfig),
42-
CoreClient: kubeExternal.Core(),
43-
AuthenticationClient: kubeExternal.Authentication(),
44-
Namespace: "kube-system",
45-
},
46-
InformerFactory: newGenericInformers(informers),
47-
Options: *controllerManagerOptions,
48-
AvailableResources: availableResources,
49-
Cloud: cloudProvider,
50-
Stop: stopCh,
51-
},
43+
OpenshiftControllerOptions: openshiftControllerOptions,
44+
5245
ClientBuilder: origincontrollers.OpenshiftControllerClientBuilder{
5346
ControllerClientBuilder: controller.SAControllerClientBuilder{
54-
ClientConfig: rest.AnonymousClientConfig(loopbackConfig),
47+
ClientConfig: rest.AnonymousClientConfig(privilegedLoopbackConfig),
5548
CoreClient: kubeExternal.Core(),
5649
AuthenticationClient: kubeExternal.Authentication(),
5750
Namespace: bootstrappolicy.DefaultOpenShiftInfraNamespace,
@@ -69,5 +62,68 @@ func getControllerContext(options configapi.MasterConfig, controllerManagerOptio
6962
Stop: stopCh,
7063
}
7164

72-
return openshiftControllerContext, nil
65+
return openshiftControllerContext
66+
}
67+
68+
// getOpenshiftControllerOptions parses the CLI args used by the kube-controllers (which control these options today), so that
69+
// we can defer making the controller options structs until we have a better idea what they should look like.
70+
// This does mean we pull in an upstream command that hopefully won't change much.
71+
func getOpenshiftControllerOptions(args map[string][]string) (origincontrollers.OpenshiftControllerOptions, error) {
72+
cmserver := cmappoptions.NewCMServer()
73+
if err := cmdflags.Resolve(args, cm.OriginControllerManagerAddFlags(cmserver)); len(err) > 0 {
74+
return origincontrollers.OpenshiftControllerOptions{}, kutilerrors.NewAggregate(err)
75+
}
76+
77+
return origincontrollers.OpenshiftControllerOptions{
78+
HPAControllerOptions: origincontrollers.HPAControllerOptions{
79+
SyncPeriod: cmserver.KubeControllerManagerConfiguration.HorizontalPodAutoscalerSyncPeriod,
80+
UpscaleForbiddenWindow: cmserver.KubeControllerManagerConfiguration.HorizontalPodAutoscalerUpscaleForbiddenWindow,
81+
DownscaleForbiddenWindow: cmserver.KubeControllerManagerConfiguration.HorizontalPodAutoscalerDownscaleForbiddenWindow,
82+
},
83+
ResourceQuotaOptions: origincontrollers.ResourceQuotaOptions{
84+
ConcurrentSyncs: cmserver.KubeControllerManagerConfiguration.ConcurrentResourceQuotaSyncs,
85+
SyncPeriod: cmserver.KubeControllerManagerConfiguration.ResourceQuotaSyncPeriod,
86+
MinResyncPeriod: cmserver.KubeControllerManagerConfiguration.MinResyncPeriod,
87+
},
88+
ServiceAccountTokenOptions: origincontrollers.ServiceAccountTokenOptions{
89+
ConcurrentSyncs: cmserver.KubeControllerManagerConfiguration.ConcurrentSATokenSyncs,
90+
},
91+
}, nil
92+
}
93+
94+
// getLeaderElectionOptions parses the CLI args used by the openshift controller leader election (which control these options today), so that
95+
// we can defer making the options structs until we have a better idea what they should look like.
96+
// This does mean we pull in an upstream command that hopefully won't change much.
97+
func getLeaderElectionOptions(args map[string][]string) (componentconfig.LeaderElectionConfiguration, error) {
98+
cmserver := cmappoptions.NewCMServer()
99+
cmserver.LeaderElection.RetryPeriod = metav1.Duration{Duration: 3 * time.Second}
100+
101+
if err := cmdflags.Resolve(args, cm.OriginControllerManagerAddFlags(cmserver)); len(err) > 0 {
102+
return componentconfig.LeaderElectionConfiguration{}, kutilerrors.NewAggregate(err)
103+
}
104+
105+
return cmserver.KubeControllerManagerConfiguration.LeaderElection, nil
106+
}
107+
108+
func waitForHealthyAPIServer(client rest.Interface) error {
109+
var healthzContent string
110+
// If apiserver is not running we should wait for some time and fail only then. This is particularly
111+
// important when we start apiserver and controller manager at the same time.
112+
err := wait.PollImmediate(time.Second, 5*time.Minute, func() (bool, error) {
113+
healthStatus := 0
114+
resp := client.Get().AbsPath("/healthz").Do().StatusCode(&healthStatus)
115+
if healthStatus != http.StatusOK {
116+
glog.Errorf("Server isn't healthy yet. Waiting a little while.")
117+
return false, nil
118+
}
119+
content, _ := resp.Raw()
120+
healthzContent = string(content)
121+
122+
return true, nil
123+
})
124+
if err != nil {
125+
return fmt.Errorf("server unhealthy: %v: %v", healthzContent, err)
126+
}
127+
128+
return nil
73129
}

0 commit comments

Comments
 (0)