diff --git a/cmd/kube-controller-manager/app/batch.go b/cmd/kube-controller-manager/app/batch.go index 1ceee260b11..ceb11e6388b 100644 --- a/cmd/kube-controller-manager/app/batch.go +++ b/cmd/kube-controller-manager/app/batch.go @@ -46,7 +46,7 @@ func startCronJobController(ctx ControllerContext) (http.Handler, bool, error) { if !ctx.AvailableResources[schema.GroupVersionResource{Group: "batch", Version: "v1beta1", Resource: "cronjobs"}] { return nil, false, nil } - cjc, err := cronjob.NewCronJobController( + cjc, err := cronjob.NewController( ctx.ClientBuilder.ClientOrDie("cronjob-controller"), ) if err != nil { diff --git a/cmd/kube-controller-manager/app/bootstrap.go b/cmd/kube-controller-manager/app/bootstrap.go index 70c00b9e52d..75e8a113f67 100644 --- a/cmd/kube-controller-manager/app/bootstrap.go +++ b/cmd/kube-controller-manager/app/bootstrap.go @@ -25,11 +25,11 @@ import ( ) func startBootstrapSignerController(ctx ControllerContext) (http.Handler, bool, error) { - bsc, err := bootstrap.NewBootstrapSigner( + bsc, err := bootstrap.NewSigner( ctx.ClientBuilder.ClientOrDie("bootstrap-signer"), ctx.InformerFactory.Core().V1().Secrets(), ctx.InformerFactory.Core().V1().ConfigMaps(), - bootstrap.DefaultBootstrapSignerOptions(), + bootstrap.DefaultSignerOptions(), ) if err != nil { return nil, true, fmt.Errorf("error creating BootstrapSigner controller: %v", err) diff --git a/hack/.golint_failures b/hack/.golint_failures index 59618617ac0..bdd2fe6bea0 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -73,14 +73,12 @@ pkg/cloudprovider/providers/photon pkg/cloudprovider/providers/vsphere pkg/controller pkg/controller/apis/config/v1alpha1 -pkg/controller/bootstrap pkg/controller/certificates pkg/controller/certificates/approver pkg/controller/certificates/signer pkg/controller/certificates/signer/config/v1alpha1 pkg/controller/cloud pkg/controller/clusterroleaggregation -pkg/controller/cronjob pkg/controller/daemon pkg/controller/daemon/config/v1alpha1 pkg/controller/deployment diff --git a/pkg/controller/bootstrap/bootstrapsigner.go b/pkg/controller/bootstrap/bootstrapsigner.go index 60952976eb5..1ae56033c7d 100644 --- a/pkg/controller/bootstrap/bootstrapsigner.go +++ b/pkg/controller/bootstrap/bootstrapsigner.go @@ -39,8 +39,8 @@ import ( "k8s.io/kubernetes/pkg/util/metrics" ) -// BootstrapSignerOptions contains options for the BootstrapSigner -type BootstrapSignerOptions struct { +// SignerOptions contains options for the Signer +type SignerOptions struct { // ConfigMapNamespace is the namespace of the ConfigMap ConfigMapNamespace string @@ -59,18 +59,17 @@ type BootstrapSignerOptions struct { SecretResync time.Duration } -// DefaultBootstrapSignerOptions returns a set of default options for creating a -// BootstrapSigner -func DefaultBootstrapSignerOptions() BootstrapSignerOptions { - return BootstrapSignerOptions{ +// DefaultSignerOptions returns a set of default options for creating a Signer. +func DefaultSignerOptions() SignerOptions { + return SignerOptions{ ConfigMapNamespace: api.NamespacePublic, ConfigMapName: bootstrapapi.ConfigMapClusterInfo, TokenSecretNamespace: api.NamespaceSystem, } } -// BootstrapSigner is a controller that signs a ConfigMap with a set of tokens. -type BootstrapSigner struct { +// Signer is a controller that signs a ConfigMap with a set of tokens. +type Signer struct { client clientset.Interface configMapKey string configMapName string @@ -90,9 +89,9 @@ type BootstrapSigner struct { configMapSynced cache.InformerSynced } -// NewBootstrapSigner returns a new *BootstrapSigner. -func NewBootstrapSigner(cl clientset.Interface, secrets informers.SecretInformer, configMaps informers.ConfigMapInformer, options BootstrapSignerOptions) (*BootstrapSigner, error) { - e := &BootstrapSigner{ +// NewSigner returns a new *Signer. +func NewSigner(cl clientset.Interface, secrets informers.SecretInformer, configMaps informers.ConfigMapInformer, options SignerOptions) (*Signer, error) { + e := &Signer{ client: cl, configMapKey: options.ConfigMapNamespace + "/" + options.ConfigMapName, configMapName: options.ConfigMapName, @@ -153,7 +152,7 @@ func NewBootstrapSigner(cl clientset.Interface, secrets informers.SecretInformer } // Run runs controller loops and returns when they are done -func (e *BootstrapSigner) Run(stopCh <-chan struct{}) { +func (e *Signer) Run(stopCh <-chan struct{}) { // Shut down queues defer utilruntime.HandleCrash() defer e.syncQueue.ShutDown() @@ -168,11 +167,11 @@ func (e *BootstrapSigner) Run(stopCh <-chan struct{}) { klog.V(1).Infof("Shutting down") } -func (e *BootstrapSigner) pokeConfigMapSync() { +func (e *Signer) pokeConfigMapSync() { e.syncQueue.Add(e.configMapKey) } -func (e *BootstrapSigner) serviceConfigMapQueue() { +func (e *Signer) serviceConfigMapQueue() { key, quit := e.syncQueue.Get() if quit { return @@ -184,7 +183,7 @@ func (e *BootstrapSigner) serviceConfigMapQueue() { // signConfigMap computes the signatures on our latest cached objects and writes // back if necessary. -func (e *BootstrapSigner) signConfigMap() { +func (e *Signer) signConfigMap() { origCM := e.getConfigMap() if origCM == nil { @@ -241,7 +240,7 @@ func (e *BootstrapSigner) signConfigMap() { } } -func (e *BootstrapSigner) updateConfigMap(cm *v1.ConfigMap) { +func (e *Signer) updateConfigMap(cm *v1.ConfigMap) { _, err := e.client.CoreV1().ConfigMaps(cm.Namespace).Update(cm) if err != nil && !apierrors.IsConflict(err) && !apierrors.IsNotFound(err) { klog.V(3).Infof("Error updating ConfigMap: %v", err) @@ -249,7 +248,7 @@ func (e *BootstrapSigner) updateConfigMap(cm *v1.ConfigMap) { } // getConfigMap gets the ConfigMap we are interested in -func (e *BootstrapSigner) getConfigMap() *v1.ConfigMap { +func (e *Signer) getConfigMap() *v1.ConfigMap { configMap, err := e.configMapLister.ConfigMaps(e.configMapNamespace).Get(e.configMapName) // If we can't get the configmap just return nil. The resync will eventually @@ -264,7 +263,7 @@ func (e *BootstrapSigner) getConfigMap() *v1.ConfigMap { return configMap } -func (e *BootstrapSigner) listSecrets() []*v1.Secret { +func (e *Signer) listSecrets() []*v1.Secret { secrets, err := e.secretLister.Secrets(e.secretNamespace).List(labels.Everything()) if err != nil { utilruntime.HandleError(err) @@ -282,7 +281,7 @@ func (e *BootstrapSigner) listSecrets() []*v1.Secret { // getTokens returns a map of tokenID->tokenSecret. It ensures the token is // valid for signing. -func (e *BootstrapSigner) getTokens() map[string]string { +func (e *Signer) getTokens() map[string]string { ret := map[string]string{} secretObjs := e.listSecrets() for _, secret := range secretObjs { diff --git a/pkg/controller/bootstrap/bootstrapsigner_test.go b/pkg/controller/bootstrap/bootstrapsigner_test.go index ce3eda7c46b..99e46861a5b 100644 --- a/pkg/controller/bootstrap/bootstrapsigner_test.go +++ b/pkg/controller/bootstrap/bootstrapsigner_test.go @@ -39,13 +39,13 @@ func init() { const testTokenID = "abc123" -func newBootstrapSigner() (*BootstrapSigner, *fake.Clientset, coreinformers.SecretInformer, coreinformers.ConfigMapInformer, error) { - options := DefaultBootstrapSignerOptions() +func newSigner() (*Signer, *fake.Clientset, coreinformers.SecretInformer, coreinformers.ConfigMapInformer, error) { + options := DefaultSignerOptions() cl := fake.NewSimpleClientset() informers := informers.NewSharedInformerFactory(fake.NewSimpleClientset(), controller.NoResyncPeriodFunc()) secrets := informers.Core().V1().Secrets() configMaps := informers.Core().V1().ConfigMaps() - bsc, err := NewBootstrapSigner(cl, secrets, configMaps, options) + bsc, err := NewSigner(cl, secrets, configMaps, options) if err != nil { return nil, nil, nil, nil, err } @@ -70,18 +70,18 @@ func newConfigMap(tokenID, signature string) *v1.ConfigMap { } func TestNoConfigMap(t *testing.T) { - signer, cl, _, _, err := newBootstrapSigner() + signer, cl, _, _, err := newSigner() if err != nil { - t.Fatalf("error creating BootstrapSigner: %v", err) + t.Fatalf("error creating Signer: %v", err) } signer.signConfigMap() verifyActions(t, []core.Action{}, cl.Actions()) } func TestSimpleSign(t *testing.T) { - signer, cl, secrets, configMaps, err := newBootstrapSigner() + signer, cl, secrets, configMaps, err := newSigner() if err != nil { - t.Fatalf("error creating BootstrapSigner: %v", err) + t.Fatalf("error creating Signer: %v", err) } cm := newConfigMap("", "") @@ -103,9 +103,9 @@ func TestSimpleSign(t *testing.T) { } func TestNoSignNeeded(t *testing.T) { - signer, cl, secrets, configMaps, err := newBootstrapSigner() + signer, cl, secrets, configMaps, err := newSigner() if err != nil { - t.Fatalf("error creating BootstrapSigner: %v", err) + t.Fatalf("error creating Signer: %v", err) } cm := newConfigMap(testTokenID, "eyJhbGciOiJIUzI1NiIsImtpZCI6ImFiYzEyMyJ9..QSxpUG7Q542CirTI2ECPSZjvBOJURUW5a7XqFpNI958") @@ -121,9 +121,9 @@ func TestNoSignNeeded(t *testing.T) { } func TestUpdateSignature(t *testing.T) { - signer, cl, secrets, configMaps, err := newBootstrapSigner() + signer, cl, secrets, configMaps, err := newSigner() if err != nil { - t.Fatalf("error creating BootstrapSigner: %v", err) + t.Fatalf("error creating Signer: %v", err) } cm := newConfigMap(testTokenID, "old signature") @@ -145,9 +145,9 @@ func TestUpdateSignature(t *testing.T) { } func TestRemoveSignature(t *testing.T) { - signer, cl, _, configMaps, err := newBootstrapSigner() + signer, cl, _, configMaps, err := newSigner() if err != nil { - t.Fatalf("error creating BootstrapSigner: %v", err) + t.Fatalf("error creating Signer: %v", err) } cm := newConfigMap(testTokenID, "old signature") diff --git a/pkg/controller/cronjob/BUILD b/pkg/controller/cronjob/BUILD index f91982bacf8..bbe121ac0e3 100644 --- a/pkg/controller/cronjob/BUILD +++ b/pkg/controller/cronjob/BUILD @@ -9,7 +9,7 @@ load( go_library( name = "go_default_library", srcs = [ - "cronjob_controller.go", + "controller.go", "doc.go", "injection.go", "utils.go", @@ -41,7 +41,7 @@ go_library( go_test( name = "go_default_test", srcs = [ - "cronjob_controller_test.go", + "controller_test.go", "utils_test.go", ], embed = [":go_default_library"], diff --git a/pkg/controller/cronjob/cronjob_controller.go b/pkg/controller/cronjob/controller.go similarity index 97% rename from pkg/controller/cronjob/cronjob_controller.go rename to pkg/controller/cronjob/controller.go index e2cb7d9ae36..6e75744fc6a 100644 --- a/pkg/controller/cronjob/cronjob_controller.go +++ b/pkg/controller/cronjob/controller.go @@ -58,7 +58,8 @@ import ( // controllerKind contains the schema.GroupVersionKind for this controller type. var controllerKind = batchv1beta1.SchemeGroupVersion.WithKind("CronJob") -type CronJobController struct { +// Controller is a controller for CronJobs. +type Controller struct { kubeClient clientset.Interface jobControl jobControlInterface sjControl sjControlInterface @@ -66,7 +67,8 @@ type CronJobController struct { recorder record.EventRecorder } -func NewCronJobController(kubeClient clientset.Interface) (*CronJobController, error) { +// NewController creates and initializes a new Controller. +func NewController(kubeClient clientset.Interface) (*Controller, error) { eventBroadcaster := record.NewBroadcaster() eventBroadcaster.StartLogging(klog.Infof) eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")}) @@ -77,7 +79,7 @@ func NewCronJobController(kubeClient clientset.Interface) (*CronJobController, e } } - jm := &CronJobController{ + jm := &Controller{ kubeClient: kubeClient, jobControl: realJobControl{KubeClient: kubeClient}, sjControl: &realSJControl{KubeClient: kubeClient}, @@ -88,8 +90,8 @@ func NewCronJobController(kubeClient clientset.Interface) (*CronJobController, e return jm, nil } -// Run the main goroutine responsible for watching and syncing jobs. -func (jm *CronJobController) Run(stopCh <-chan struct{}) { +// Run starts the main goroutine responsible for watching and syncing jobs. +func (jm *Controller) Run(stopCh <-chan struct{}) { defer utilruntime.HandleCrash() klog.Infof("Starting CronJob Manager") // Check things every 10 second. @@ -99,7 +101,7 @@ func (jm *CronJobController) Run(stopCh <-chan struct{}) { } // syncAll lists all the CronJobs and Jobs and reconciles them. -func (jm *CronJobController) syncAll() { +func (jm *Controller) syncAll() { // List children (Jobs) before parents (CronJob). // This guarantees that if we see any Job that got orphaned by the GC orphan finalizer, // we must also see that the parent CronJob has non-nil DeletionTimestamp (see #42639). diff --git a/pkg/controller/cronjob/cronjob_controller_test.go b/pkg/controller/cronjob/controller_test.go similarity index 100% rename from pkg/controller/cronjob/cronjob_controller_test.go rename to pkg/controller/cronjob/controller_test.go diff --git a/pkg/controller/cronjob/utils.go b/pkg/controller/cronjob/utils.go index 19fb91c4baa..8b777f02b35 100644 --- a/pkg/controller/cronjob/utils.go +++ b/pkg/controller/cronjob/utils.go @@ -142,7 +142,7 @@ func getRecentUnmetScheduleTimes(sj batchv1beta1.CronJob, now time.Time) ([]time // but less than "lots". if len(starts) > 100 { // We can't get the most recent times so just return an empty slice - return []time.Time{}, fmt.Errorf("Too many missed start time (> 100). Set or decrease .spec.startingDeadlineSeconds or check clock skew.") + return []time.Time{}, fmt.Errorf("too many missed start time (> 100). Set or decrease .spec.startingDeadlineSeconds or check clock skew") } } return starts, nil @@ -183,6 +183,7 @@ func getFinishedStatus(j *batchv1.Job) (bool, batchv1.JobConditionType) { return false, "" } +// IsJobFinished returns whether or not a job has completed successfully or failed. func IsJobFinished(j *batchv1.Job) bool { isFinished, _ := getFinishedStatus(j) return isFinished diff --git a/pkg/controller/cronjob/utils_test.go b/pkg/controller/cronjob/utils_test.go index fb6b569f2e6..f9b041f81b7 100644 --- a/pkg/controller/cronjob/utils_test.go +++ b/pkg/controller/cronjob/utils_test.go @@ -35,7 +35,7 @@ func TestGetJobFromTemplate(t *testing.T) { // and other fields, and add a created-by reference. var one int64 = 1 - var no bool = false + var no bool sj := batchv1beta1.CronJob{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/integration/cronjob/cronjob_test.go b/test/integration/cronjob/cronjob_test.go index 1a719a6b934..7d496fbf85b 100644 --- a/test/integration/cronjob/cronjob_test.go +++ b/test/integration/cronjob/cronjob_test.go @@ -38,7 +38,7 @@ import ( "k8s.io/kubernetes/test/integration/framework" ) -func setup(t *testing.T) (*httptest.Server, framework.CloseFunc, *cronjob.CronJobController, *job.JobController, informers.SharedInformerFactory, clientset.Interface, rest.Config) { +func setup(t *testing.T) (*httptest.Server, framework.CloseFunc, *cronjob.Controller, *job.JobController, informers.SharedInformerFactory, clientset.Interface, rest.Config) { masterConfig := framework.NewIntegrationTestMasterConfig() _, server, closeFn := framework.RunAMaster(masterConfig) @@ -49,7 +49,7 @@ func setup(t *testing.T) (*httptest.Server, framework.CloseFunc, *cronjob.CronJo } resyncPeriod := 12 * time.Hour informerSet := informers.NewSharedInformerFactory(clientset.NewForConfigOrDie(restclient.AddUserAgent(&config, "cronjob-informers")), resyncPeriod) - cjc, err := cronjob.NewCronJobController(clientSet) + cjc, err := cronjob.NewController(clientSet) if err != nil { t.Fatalf("Error creating CronJob controller: %v", err) }