Repository: furiko-io/furiko
Pull request #42 (2022-04-11 05:22:36)

refactor(config): Update dynamic config type names
Updates config names in the code, as well as rename ConfigMap file names. List of changes are below.

- `execution-job` -> `jobs`
- `execution-cron` -> `cron`

With this, we can just specify the short name in the ConfigMap itself, like so:

```yaml
apiVersion: v1
kind: ConfigMap
data:
  _readme: |
    # This ConfigMap contains the dynamic config for execution-controller.
    # We can tune several knobs in execution-controller without requiring a restart.
    # Each file in this ConfigMap groups together configuration of a single sub-component.
    # As a start, we have populated a set of sane default values for you.
    # More info: https://furiko.io/reference/configuration/dynamic/
  cron: |
    apiVersion: config.furiko.io/v1alpha1
    kind: CronExecutionConfig
    cronFormat: "standard"
    cronHashNames: true
    cronHashSecondsByDefault: false
    cronHashFields: true
    defaultTimezone: "UTC"
    maxMissedSchedules: 5
    maxDowntimeThresholdSeconds: 300
  jobs: |
    apiVersion: config.furiko.io/v1alpha1
    kind: JobExecutionConfig
    defaultTTLSecondsAfterFinished: 3600
    defaultPendingTimeoutSeconds: 900
    deleteKillingTasksTimeoutSeconds: 180
    forceDeleteKillingTasksTimeoutSeconds: 120
 ```


Patch:
diff --git a/Makefile b/Makefile
index d39cf97..fbfff9b 100644
--- a/Makefile
+++ b/Makefile
@@ -72,8 +72,7 @@ generate-groups: generate-groups.sh ## Generate code such as client, lister, inf
 
 .PHONY: fmt
 fmt: goimports ## Format code.
-	go fmt ./...
-	$(GOIMPORTS) -w -local "$(PKG)" .
+	./hack/run-fmt.sh "$(PKG)"
 
 .PHONY: lint
 lint: lint-go lint-license ## Lint all code.
diff --git a/apis/config/v1alpha1/configurationname_types.go b/apis/config/v1alpha1/configurationname_types.go
index b030597..6138455 100644
--- a/apis/config/v1alpha1/configurationname_types.go
+++ b/apis/config/v1alpha1/configurationname_types.go
@@ -21,8 +21,8 @@ type ConfigName string
 
 const (
 	// JobExecutionConfigName refers to JobExecutionConfig.
-	JobExecutionConfigName ConfigName = "execution-job"
+	JobExecutionConfigName ConfigName = "jobs"
 
 	// CronExecutionConfigName refers to CronExecutionConfig.
-	CronExecutionConfigName ConfigName = "execution-cron"
+	CronExecutionConfigName ConfigName = "cron"
 )
diff --git a/config/execution/dynamic/config.yaml b/config/execution/dynamic/config.yaml
index 5c87f4f..c39ec81 100644
--- a/config/execution/dynamic/config.yaml
+++ b/config/execution/dynamic/config.yaml
@@ -3,9 +3,14 @@ kind: ConfigMap
 metadata:
   name: dynamic-config
 data:
-  execution-job: |
-    # Here we define the dynamic config for execution-controller without requiring
-    # a restart. This config file configures dynamic configuration for Jobs.
+  _readme: |
+    # This ConfigMap contains the dynamic config for execution-controller.
+    # We can tune several knobs in execution-controller without requiring a restart.
+    # Each file in this ConfigMap groups together configuration of a single sub-component.
+    # As a start, we have populated a set of sane default values for you.
+    # More info: https://furiko.io/reference/configuration/dynamic/
+
+  jobs: |
     apiVersion: config.furiko.io/v1alpha1
     kind: JobExecutionConfig
 
@@ -30,9 +35,7 @@ data:
     # of deletionGracePeriodSeconds. Set this value to 0 to disable force deletion.
     forceDeleteKillingTasksTimeoutSeconds: 120
 
-  execution-cron: |
-    # Here we define the dynamic config for execution-controller without requiring
-    # a restart. This config file configures dynamic configuration for Cron.
+  cron: |
     apiVersion: config.furiko.io/v1alpha1
     kind: CronExecutionConfig
 
diff --git a/hack/run-fmt.sh b/hack/run-fmt.sh
new file mode 100755
index 0000000..edf3f74
--- /dev/null
+++ b/hack/run-fmt.sh
@@ -0,0 +1,50 @@
+#!/bin/bash
+# shellcheck disable=SC2046
+
+#
+# Copyright 2022 The Furiko Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -euo pipefail
+
+## Simple script that performs code formatting.
+
+if [ $# -ne 1 ]
+then
+  echo 'Usage:'
+  echo '  ./run-fmt.sh GO_PACKAGE_NAME'
+  echo
+  echo 'Optional environment variables:'
+  echo '  GOIMPORTS: Path to goimports executable. Default: ./bin/goimports'
+  exit 1
+fi
+
+# Positional arguments
+GO_PACKAGE_NAME="$1"
+if [[ -z "${GO_PACKAGE_NAME}" ]]
+then
+  echo 'Error: GO_PACKAGE_NAME cannot be empty'
+  exit 2
+fi
+
+# Optional environment variables
+GOIMPORTS="${GOIMPORTS:-$(pwd)/bin/goimports}"
+
+# Run go fmt.
+go fmt ./...
+
+# Run goimports to sort imports.
+# Do not sort generated files.
+./bin/goimports -w -local "${GO_PACKAGE_NAME}" ./cmd $(go list -f "{{.Dir}}" ./pkg/... | grep -v pkg/generated)
diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go
index 34d875b..0916b28 100644
--- a/pkg/config/defaults.go
+++ b/pkg/config/defaults.go
@@ -23,14 +23,14 @@ import (
 )
 
 var (
-	DefaultJobControllerConfig = &configv1alpha1.JobExecutionConfig{
+	DefaultJobExecutionConfig = &configv1alpha1.JobExecutionConfig{
 		DefaultTTLSecondsAfterFinished:        pointer.Int64(3600),
 		DefaultPendingTimeoutSeconds:          pointer.Int64(900),
 		DeleteKillingTasksTimeoutSeconds:      pointer.Int64(180),
 		ForceDeleteKillingTasksTimeoutSeconds: pointer.Int64(120),
 	}
 
-	DefaultCronControllerConfig = &configv1alpha1.CronExecutionConfig{
+	DefaultCronExecutionConfig = &configv1alpha1.CronExecutionConfig{
 		CronFormat:                  "standard",
 		CronHashNames:               pointer.Bool(true),
 		CronHashSecondsByDefault:    pointer.Bool(false),
diff --git a/pkg/execution/controllers/croncontroller/cron_worker.go b/pkg/execution/controllers/croncontroller/cron_worker.go
index 678c09e..9fdfce9 100644
--- a/pkg/execution/controllers/croncontroller/cron_worker.go
+++ b/pkg/execution/controllers/croncontroller/cron_worker.go
@@ -84,7 +84,7 @@ func (w *CronWorker) Work() {
 	defer trace.LogIfLong(CronWorkerInterval / 2)
 
 	// Load dynamic configuration.
-	cfg, err := w.Configs().CronController()
+	cfg, err := w.Configs().Cron()
 	if err != nil {
 		klog.ErrorS(err, "croncontroller: cannot load controller configuration")
 		return
diff --git a/pkg/execution/controllers/croncontroller/schedule.go b/pkg/execution/controllers/croncontroller/schedule.go
index 6dd2408..9c7dda7 100644
--- a/pkg/execution/controllers/croncontroller/schedule.go
+++ b/pkg/execution/controllers/croncontroller/schedule.go
@@ -65,7 +65,7 @@ func (w *Schedule) GetNextScheduleTime(
 	timezone := fromTime.Location()
 
 	maxDowntimeThreshold := defaultMaxDowntimeThreshold
-	if cfg, err := w.ctrlContext.Configs().CronController(); err == nil && cfg.MaxDowntimeThresholdSeconds > 0 {
+	if cfg, err := w.ctrlContext.Configs().Cron(); err == nil && cfg.MaxDowntimeThresholdSeconds > 0 {
 		maxDowntimeThreshold = time.Second * time.Duration(cfg.MaxDowntimeThresholdSeconds)
 	}
 
diff --git a/pkg/execution/controllers/jobcontroller/reconciler.go b/pkg/execution/controllers/jobcontroller/reconciler.go
index 72c5ea7..0a74247 100644
--- a/pkg/execution/controllers/jobcontroller/reconciler.go
+++ b/pkg/execution/controllers/jobcontroller/reconciler.go
@@ -72,7 +72,7 @@ func (w *Reconciler) MaxRequeues() int {
 func (w *Reconciler) SyncOne(ctx context.Context, namespace, name string, _ int) error {
 	var err error
 
-	cfg, err := w.Configs().JobController()
+	cfg, err := w.Configs().Jobs()
 	if err != nil {
 		return errors.Wrapf(err, "cannot load controller configuration")
 	}
diff --git a/pkg/execution/mutation/mutation.go b/pkg/execution/mutation/mutation.go
index 206cdf7..2618d46 100644
--- a/pkg/execution/mutation/mutation.go
+++ b/pkg/execution/mutation/mutation.go
@@ -124,7 +124,7 @@ func (m *Mutator) MutateUpdateJobConfig(oldRjc, rjc *v1alpha1.JobConfig) *webhoo
 func (m *Mutator) MutateJob(rj *v1alpha1.Job) *webhook.Result {
 	result := webhook.NewResult()
 
-	cfg, err := m.ctrlContext.Configs().JobController()
+	cfg, err := m.ctrlContext.Configs().Jobs()
 	if err != nil {
 		result.Errors = append(result.Errors, field.InternalError(field.NewPath(""), err))
 	}
diff --git a/pkg/execution/validation/validation.go b/pkg/execution/validation/validation.go
index b3eb1d4..4b04606 100644
--- a/pkg/execution/validation/validation.go
+++ b/pkg/execution/validation/validation.go
@@ -237,8 +237,8 @@ func (v *Validator) ValidateConcurrencyPolicy(concurrencyPolicy v1alpha1.Concurr
 func (v *Validator) ValidateCronScheduleExpression(cronSchedule string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	// Load the CronController config to determine how to parse the cron expression.
-	cfg, err := v.ctrlContext.Configs().CronController()
+	// Load the Cron config to determine how to parse the cron expression.
+	cfg, err := v.ctrlContext.Configs().Cron()
 	if err != nil {
 		allErrs = append(allErrs, field.InternalError(fldPath, errors.Wrapf(err, "cannot load cron config")))
 		return allErrs
diff --git a/pkg/runtime/configloader/defaults_loader.go b/pkg/runtime/configloader/defaults_loader.go
index 298eeb2..947a9f1 100644
--- a/pkg/runtime/configloader/defaults_loader.go
+++ b/pkg/runtime/configloader/defaults_loader.go
@@ -43,8 +43,8 @@ var _ Loader = (*DefaultsLoader)(nil)
 func NewDefaultsLoader() *DefaultsLoader {
 	return &DefaultsLoader{
 		Defaults: map[configv1alpha1.ConfigName]runtime.Object{
-			configv1alpha1.JobExecutionConfigName:  config.DefaultJobControllerConfig,
-			configv1alpha1.CronExecutionConfigName: config.DefaultCronControllerConfig,
+			configv1alpha1.JobExecutionConfigName:  config.DefaultJobExecutionConfig,
+			configv1alpha1.CronExecutionConfigName: config.DefaultCronExecutionConfig,
 		},
 	}
 }
diff --git a/pkg/runtime/controllercontext/context_configs.go b/pkg/runtime/controllercontext/context_configs.go
index 17b8f75..4f1b39e 100644
--- a/pkg/runtime/controllercontext/context_configs.go
+++ b/pkg/runtime/controllercontext/context_configs.go
@@ -27,6 +27,9 @@ import (
 	"github.com/furiko-io/furiko/pkg/runtime/configloader"
 )
 
+// ConfigsMap is a map of ConfigName to Config object.
+type ConfigsMap = map[configv1alpha1.ConfigName]runtime.Object
+
 // Configs returns the dynamic controller configurations.
 func (c *ctrlContext) Configs() Configs {
 	return c.configMgr
@@ -35,8 +38,8 @@ func (c *ctrlContext) Configs() Configs {
 type Configs interface {
 	Start(ctx context.Context) error
 	AllConfigs() (map[configv1alpha1.ConfigName]runtime.Object, error)
-	JobController() (*configv1alpha1.JobExecutionConfig, error)
-	CronController() (*configv1alpha1.CronExecutionConfig, error)
+	Jobs() (*configv1alpha1.JobExecutionConfig, error)
+	Cron() (*configv1alpha1.CronExecutionConfig, error)
 }
 
 type ContextConfigs struct {
@@ -51,10 +54,10 @@ func NewContextConfigs(mgr *configloader.ConfigManager) *ContextConfigs {
 func (c *ContextConfigs) AllConfigs() (map[configv1alpha1.ConfigName]runtime.Object, error) {
 	configNameMap := map[configv1alpha1.ConfigName]func() (runtime.Object, error){
 		configv1alpha1.JobExecutionConfigName: func() (runtime.Object, error) {
-			return c.JobController()
+			return c.Jobs()
 		},
 		configv1alpha1.CronExecutionConfigName: func() (runtime.Object, error) {
-			return c.CronController()
+			return c.Cron()
 		},
 	}
 
@@ -70,8 +73,8 @@ func (c *ContextConfigs) AllConfigs() (map[configv1alpha1.ConfigName]runtime.Obj
 	return configs, nil
 }
 
-// JobController returns the job controller configuration.
-func (c *ContextConfigs) JobController() (*configv1alpha1.JobExecutionConfig, error) {
+// Jobs returns the job dynamic configuration.
+func (c *ContextConfigs) Jobs() (*configv1alpha1.JobExecutionConfig, error) {
 	var config configv1alpha1.JobExecutionConfig
 	if err := c.LoadAndUnmarshalConfig(configv1alpha1.JobExecutionConfigName, &config); err != nil {
 		return nil, err
@@ -79,8 +82,8 @@ func (c *ContextConfigs) JobController() (*configv1alpha1.JobExecutionConfig, er
 	return &config, nil
 }
 
-// CronController returns the cron controller configuration.
-func (c *ContextConfigs) CronController() (*configv1alpha1.CronExecutionConfig, error) {
+// Cron returns the cron dynamic configuration.
+func (c *ContextConfigs) Cron() (*configv1alpha1.CronExecutionConfig, error) {
 	var config configv1alpha1.CronExecutionConfig
 	if err := c.LoadAndUnmarshalConfig(configv1alpha1.CronExecutionConfigName, &config); err != nil {
 		return nil, err


Test patch:
diff --git a/pkg/execution/controllers/croncontroller/schedule_test.go b/pkg/execution/controllers/croncontroller/schedule_test.go
index c11f8ba..b93fc5c 100644
--- a/pkg/execution/controllers/croncontroller/schedule_test.go
+++ b/pkg/execution/controllers/croncontroller/schedule_test.go
@@ -274,7 +274,7 @@ func TestSchedule(t *testing.T) {
 				t.Fatalf("cannot start context: %v", err)
 			}
 
-			cfg, err := ctrlContext.Configs().CronController()
+			cfg, err := ctrlContext.Configs().Cron()
 			if err != nil {
 				t.Fatalf("cannot get controller configuration: %v", err)
 			}
@@ -318,7 +318,7 @@ func TestSchedule_FlushNextScheduleTime(t *testing.T) {
 		t.Fatalf("cannot start context: %v", err)
 	}
 
-	cfg, err := ctrlContext.Configs().CronController()
+	cfg, err := ctrlContext.Configs().Cron()
 	if err != nil {
 		t.Fatalf("cannot get controller configuration: %v", err)
 	}
diff --git a/pkg/execution/controllers/jobcontroller/controller_test.go b/pkg/execution/controllers/jobcontroller/controller_test.go
index c79dfe7..9fc383d 100644
--- a/pkg/execution/controllers/jobcontroller/controller_test.go
+++ b/pkg/execution/controllers/jobcontroller/controller_test.go
@@ -256,7 +256,7 @@ var (
 	fakePodDeleting = func() *corev1.Pod {
 		newPod := fakePodTerminating.DeepCopy()
 		deleteTime := metav1.NewTime(testutils.Mkmtimep(killTime).
-			Add(time.Duration(*config.DefaultJobControllerConfig.DeleteKillingTasksTimeoutSeconds) * time.Second))
+			Add(time.Duration(*config.DefaultJobExecutionConfig.DeleteKillingTasksTimeoutSeconds) * time.Second))
 		newPod.DeletionTimestamp = &deleteTime
 		return newPod
 	}()
diff --git a/pkg/execution/controllers/jobcontroller/reconciler_test.go b/pkg/execution/controllers/jobcontroller/reconciler_test.go
index 1e37c3a..4d7fbb3 100644
--- a/pkg/execution/controllers/jobcontroller/reconciler_test.go
+++ b/pkg/execution/controllers/jobcontroller/reconciler_test.go
@@ -205,7 +205,7 @@ func TestReconciler(t *testing.T) {
 		{
 			name: "delete pod with kill timestamp",
 			now: testutils.Mktime(killTime).
-				Add(time.Duration(*config.DefaultJobControllerConfig.DeleteKillingTasksTimeoutSeconds) * time.Second),
+				Add(time.Duration(*config.DefaultJobExecutionConfig.DeleteKillingTasksTimeoutSeconds) * time.Second),
 			target: fakeJobWithKillTimestamp,
 			initialPods: []*corev1.Pod{
 				fakePodTerminating,
@@ -224,8 +224,8 @@ func TestReconciler(t *testing.T) {
 		{
 			name: "force delete pod with kill timestamp",
 			now: testutils.Mktime(killTime).
-				Add(time.Duration(*config.DefaultJobControllerConfig.DeleteKillingTasksTimeoutSeconds) * time.Second).
-				Add(time.Duration(*config.DefaultJobControllerConfig.ForceDeleteKillingTasksTimeoutSeconds) * time.Second),
+				Add(time.Duration(*config.DefaultJobExecutionConfig.DeleteKillingTasksTimeoutSeconds) * time.Second).
+				Add(time.Duration(*config.DefaultJobExecutionConfig.ForceDeleteKillingTasksTimeoutSeconds) * time.Second),
 			target: fakeJobPodDeleting,
 			initialPods: []*corev1.Pod{
 				fakePodDeleting,
@@ -244,8 +244,8 @@ func TestReconciler(t *testing.T) {
 		{
 			name: "do not force delete pod if disabled via config",
 			now: testutils.Mktime(killTime).
-				Add(time.Duration(*config.DefaultJobControllerConfig.DeleteKillingTasksTimeoutSeconds) * time.Second).
-				Add(time.Duration(*config.DefaultJobControllerConfig.ForceDeleteKillingTasksTimeoutSeconds) * time.Second),
+				Add(time.Duration(*config.DefaultJobExecutionConfig.DeleteKillingTasksTimeoutSeconds) * time.Second).
+				Add(time.Duration(*config.DefaultJobExecutionConfig.ForceDeleteKillingTasksTimeoutSeconds) * time.Second),
 			target: fakeJobPodDeleting,
 			initialPods: []*corev1.Pod{
 				fakePodDeleting,
@@ -259,8 +259,8 @@ func TestReconciler(t *testing.T) {
 		{
 			name: "do not force delete pod if disabled via JobSpec",
 			now: testutils.Mktime(killTime).
-				Add(time.Duration(*config.DefaultJobControllerConfig.DeleteKillingTasksTimeoutSeconds) * time.Second).
-				Add(time.Duration(*config.DefaultJobControllerConfig.ForceDeleteKillingTasksTimeoutSeconds) * time.Second),
+				Add(time.Duration(*config.DefaultJobExecutionConfig.DeleteKillingTasksTimeoutSeconds) * time.Second).
+				Add(time.Duration(*config.DefaultJobExecutionConfig.ForceDeleteKillingTasksTimeoutSeconds) * time.Second),
 			target: fakeJobPodDeletingForbidForceDeletion,
 			initialPods: []*corev1.Pod{
 				fakePodDeleting,
@@ -327,7 +327,7 @@ func TestReconciler(t *testing.T) {
 		{
 			name: "delete finished job on TTL after finished",
 			now: testutils.Mktime(finishTime).
-				Add(time.Duration(*config.DefaultJobControllerConfig.DefaultTTLSecondsAfterFinished) * time.Second),
+				Add(time.Duration(*config.DefaultJobExecutionConfig.DefaultTTLSecondsAfterFinished) * time.Second),
 			target: fakeJobFinished,
 			initialPods: []*corev1.Pod{
 				fakePodFinished,
diff --git a/pkg/execution/mutation/mutation_test.go b/pkg/execution/mutation/mutation_test.go
index d9b65c4..bee898e 100644
--- a/pkg/execution/mutation/mutation_test.go
+++ b/pkg/execution/mutation/mutation_test.go
@@ -603,7 +603,7 @@ func TestMutator_MutateJob(t *testing.T) {
 				Spec: v1alpha1.JobSpec{
 					Type:                    v1alpha1.JobTypeAdhoc,
 					Template:                &jobTemplateSpecBasic.Spec,
-					TTLSecondsAfterFinished: config.DefaultJobControllerConfig.DefaultTTLSecondsAfterFinished,
+					TTLSecondsAfterFinished: config.DefaultJobExecutionConfig.DefaultTTLSecondsAfterFinished,
 				},
 			},
 		},
@@ -614,7 +614,7 @@ func TestMutator_MutateJob(t *testing.T) {
 				Spec: v1alpha1.JobSpec{
 					Type:                    v1alpha1.JobTypeAdhoc,
 					Template:                &jobTemplateSpecBasic.Spec,
-					TTLSecondsAfterFinished: config.DefaultJobControllerConfig.DefaultTTLSecondsAfterFinished,
+					TTLSecondsAfterFinished: config.DefaultJobExecutionConfig.DefaultTTLSecondsAfterFinished,
 				},
 			},
 		},