Browse Source

Attribute batch/v1 CronJob controller correctly

As explained in the new comments, batch/v1beta1 CronJobs create Jobs with
a 10 character timestamp at the end. Our regex (and therefore Pod ->
Job -> CronJob controller attribution) works for this case. However,
batch/v1 CronJobs apparently create Jobs with 8 character timestamps.
This breaks our controller attribution, resulting in every Job being its
own controller.

This commit updates our regex (and adds a unit test) to support the
batch/v1 CronJob Job name format.

I pulled test Job names from live clusters, one with a batch/v1 CronJob
and another with a batch/v1beta1 CronJob. GKE wouldn't let me create a
new batch/v1beta1 CronJob -- it seems to auto-upgrade. Fortunately, there
already was a batch/v1beta1 CronJob in our integration test cluster.
Michael Dresser 4 years ago
parent
commit
7d4c70c590
2 changed files with 72 additions and 1 deletions
  1. 5 1
      pkg/costmodel/costmodel.go
  2. 67 0
      pkg/costmodel/costmodel_test.go

+ 5 - 1
pkg/costmodel/costmodel.go

@@ -44,7 +44,11 @@ const (
 )
 
 // isCron matches a CronJob name and captures the non-timestamp name
-var isCron = regexp.MustCompile(`^(.+)-\d{10}$`)
+//
+// We support either a 10 character timestamp OR an 8 character timestamp
+// because batch/v1beta1 CronJobs creates Jobs with 10 character timestamps
+// and batch/v1 CronJobs create Jobs with 8 character timestamps.
+var isCron = regexp.MustCompile(`^(.+)-(\d{10}|\d{8})$`)
 
 type CostModel struct {
 	Cache                      clustercache.ClusterCache

+ 67 - 0
pkg/costmodel/costmodel_test.go

@@ -0,0 +1,67 @@
+package costmodel
+
+import (
+	"testing"
+)
+
+func Test_CostData_GetController_CronJob(t *testing.T) {
+	cases := []struct {
+		name string
+		cd   CostData
+
+		expectedName          string
+		expectedKind          string
+		expectedHasController bool
+	}{
+		{
+			name: "batch/v1beta1 CronJob Job name",
+			cd: CostData{
+				// batch/v1beta1 CronJobs create Jobs with a 10 character
+				// timestamp appended to the end of the name.
+				//
+				// It looks like this:
+				// CronJob: cronjob-1
+				// Job: cronjob-1-1651057200
+				// Pod: cronjob-1-1651057200-mf5c9
+				Jobs: []string{"cronjob-1-1651057200"},
+			},
+
+			expectedName:          "cronjob-1",
+			expectedKind:          "job",
+			expectedHasController: true,
+		},
+		{
+			name: "batch/v1 CronJob Job name",
+			cd: CostData{
+				// batch/v1CronJobs create Jobs with an 8 character timestamp
+				// appended to the end of the name.
+				//
+				// It looks like this:
+				// CronJob: cj-v1
+				// Job: cj-v1-27517770
+				// Pod: cj-v1-27517770-xkrgn
+				Jobs: []string{"cj-v1-27517770"},
+			},
+
+			expectedName:          "cj-v1",
+			expectedKind:          "job",
+			expectedHasController: true,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			name, kind, hasController := c.cd.GetController()
+
+			if name != c.expectedName {
+				t.Errorf("Name mismatch. Expected: %s. Got: %s", c.expectedName, name)
+			}
+			if kind != c.expectedKind {
+				t.Errorf("Kind mismatch. Expected: %s. Got: %s", c.expectedKind, kind)
+			}
+			if hasController != c.expectedHasController {
+				t.Errorf("HasController mismatch. Expected: %t. Got: %t", c.expectedHasController, hasController)
+			}
+		})
+	}
+}