diff --git a/Makefile b/Makefile index 66d2f8edb..c5efaecf5 100644 --- a/Makefile +++ b/Makefile @@ -433,9 +433,9 @@ hack-deploy: --set worker.image.repository=$(DOCKER_IMAGE_PREFIX)worker \ --set worker.image.tag=$(IMMUTABLE_DOCKER_TAG) \ --set worker.image.pullPolicy=$(IMAGE_PULL_POLICY) \ - --set gitInitializer.image.repository=$(DOCKER_IMAGE_PREFIX)git-initializer \ - --set gitInitializer.image.tag=$(IMMUTABLE_DOCKER_TAG) \ - --set gitInitializer.image.pullPolicy=$(IMAGE_PULL_POLICY) \ + --set gitInitializer.linux.image.repository=$(DOCKER_IMAGE_PREFIX)git-initializer \ + --set gitInitializer.linux.image.tag=$(IMMUTABLE_DOCKER_TAG) \ + --set gitInitializer.linux.image.pullPolicy=$(IMAGE_PULL_POLICY) \ --set logger.linux.image.repository=$(DOCKER_IMAGE_PREFIX)logger\ --set logger.linux.image.tag=$(IMMUTABLE_DOCKER_TAG) \ --set logger.linux.image.pullPolicy=$(IMAGE_PULL_POLICY) diff --git a/charts/brigade/templates/apiserver/deployment.yaml b/charts/brigade/templates/apiserver/deployment.yaml index 87e8ca2ae..0414ee3ad 100644 --- a/charts/brigade/templates/apiserver/deployment.yaml +++ b/charts/brigade/templates/apiserver/deployment.yaml @@ -150,9 +150,13 @@ spec: name: {{ include "brigade.artemis.fullname" . }} key: password - name: GIT_INITIALIZER_IMAGE - value: {{ .Values.gitInitializer.image.repository }}:{{ default .Chart.AppVersion .Values.gitInitializer.image.tag }} + value: {{ .Values.gitInitializer.linux.image.repository }}:{{ default .Chart.AppVersion .Values.gitInitializer.linux.image.tag }} - name: GIT_INITIALIZER_IMAGE_PULL_POLICY - value: {{ .Values.gitInitializer.image.pullPolicy }} + value: {{ .Values.gitInitializer.linux.image.pullPolicy }} + - name: GIT_INITIALIZER_WINDOWS_IMAGE + value: {{ .Values.gitInitializer.windows.image.repository }}:{{ default .Chart.AppVersion .Values.gitInitializer.windows.image.tag }} + - name: GIT_INITIALIZER_WINDOWS_IMAGE_PULL_POLICY + value: {{ .Values.gitInitializer.windows.image.pullPolicy }} - name: DEFAULT_WORKER_IMAGE value: {{ .Values.worker.image.repository }}:{{ default .Chart.AppVersion .Values.worker.image.tag }} - name: DEFAULT_WORKER_IMAGE_PULL_POLICY diff --git a/charts/brigade/values.yaml b/charts/brigade/values.yaml index 866403cdf..b281681a8 100644 --- a/charts/brigade/values.yaml +++ b/charts/brigade/values.yaml @@ -253,12 +253,21 @@ observer: gitInitializer: - image: - repository: brigadecore/brigade2-git-initializer - ## tag should only be specified if you want to override Chart.appVersion - ## The default tag is the value of .Chart.AppVersion - # tag: - pullPolicy: IfNotPresent + linux: + image: + repository: brigadecore/brigade2-git-initializer + ## tag should only be specified if you want to override Chart.appVersion + ## The default tag is the value of .Chart.AppVersion + # tag: + pullPolicy: IfNotPresent + + windows: + image: + repository: brigadecore/brigade2-git-initializer-windows + ## tag should only be specified if you want to override Chart.appVersion + ## The default tag is the value of .Chart.AppVersion + # tag: + pullPolicy: IfNotPresent worker: diff --git a/sdk/v2/core/jobs.go b/sdk/v2/core/jobs.go index 389abf767..2dc74ea6b 100644 --- a/sdk/v2/core/jobs.go +++ b/sdk/v2/core/jobs.go @@ -16,6 +16,16 @@ import ( // JobKind represents the canonical Job kind string const JobKind = "Job" +// OSFamily represents a type of operating system. +type OSFamily string + +const ( + // OSFamilyLinux represents a Linux-based OS. + OSFamilyLinux OSFamily = "linux" + // OSFamilyWindows represents a Windows-based OS. + OSFamilyWindows OSFamily = "windows" +) + // JobPhase represents where a Job is within its lifecycle. type JobPhase string @@ -182,7 +192,7 @@ type JobHost struct { // OS specifies which "family" of operating system is required on a substrate // node to host a Job. Valid values are "linux" and "windows". When empty, // Brigade assumes "linux". - OS string `json:"os,omitempty"` + OS OSFamily `json:"os,omitempty"` // NodeSelector specifies labels that must be present on the substrate node to // host a Job. This provides an opaque mechanism for communicating Job needs // such as specific hardware like an SSD or GPU. diff --git a/v2/apiserver/config.go b/v2/apiserver/config.go index c5adf33c6..2612b3a94 100644 --- a/v2/apiserver/config.go +++ b/v2/apiserver/config.go @@ -119,6 +119,18 @@ func substrateConfig() (kubernetes.SubstrateConfig, error) { } config.GitInitializerImagePullPolicy = api.ImagePullPolicy(gitInitializerImagePullPolicyStr) + config.GitInitializerWindowsImage, err = + os.GetRequiredEnvVar("GIT_INITIALIZER_WINDOWS_IMAGE") + if err != nil { + return config, err + } + gitInitializerWindowsImagePullPolicyStr, err := + os.GetRequiredEnvVar("GIT_INITIALIZER_WINDOWS_IMAGE_PULL_POLICY") + if err != nil { + return config, err + } + config.GitInitializerWindowsImagePullPolicy = + api.ImagePullPolicy(gitInitializerWindowsImagePullPolicyStr) config.DefaultWorkerImage, err = os.GetRequiredEnvVar("DEFAULT_WORKER_IMAGE") if err != nil { return config, err diff --git a/v2/apiserver/config_test.go b/v2/apiserver/config_test.go index 6cb815b13..890759bc4 100644 --- a/v2/apiserver/config_test.go +++ b/v2/apiserver/config_test.go @@ -158,13 +158,18 @@ func TestWriterFactoryConfig(t *testing.T) { } func TestSubstrateConfig(t *testing.T) { - const testBrigadeID = "4077th" - const testAPIAddress = "http://localhost" - const testGitInitializerImage = "brigadecore/brigade2-git-initializer:2.0.0" - const testGitInitializerImagePullPolicy = api.ImagePullPolicy("IfNotPresent") - const testDefaultWorkerImage = "brigadecore/brigade2-worker:2.0.0" - const testDefaultWorkerImagePullPolicy = api.ImagePullPolicy("IfNotPresent") - const testWorkspaceStorageClass = "nfs" + // nolint: lll + const ( + testBrigadeID = "4077th" + testAPIAddress = "http://localhost" + testGitInitializerImage = "brigadecore/brigade2-git-initializer:2.0.0" + testGitInitializerImagePullPolicy = api.ImagePullPolicy("IfNotPresent") + testGitInitializerWindowsImage = "brigadecore/brigade2-git-initializer-windows:2.0.0" + testGitInitializerWindowsImagePullPolicy = api.ImagePullPolicy("IfNotPresent") + testDefaultWorkerImage = "brigadecore/brigade2-worker:2.0.0" + testDefaultWorkerImagePullPolicy = api.ImagePullPolicy("IfNotPresent") + testWorkspaceStorageClass = "nfs" + ) testCases := []struct { name string setup func() @@ -213,13 +218,45 @@ func TestSubstrateConfig(t *testing.T) { }, }, { - name: "DEFAULT_WORKER_IMAGE not set", + name: "GIT_INITIALIZER_WINDOWS_IMAGE not set", setup: func() { t.Setenv( "GIT_INITIALIZER_IMAGE_PULL_POLICY", string(testGitInitializerImagePullPolicy), ) }, + assertions: func(_ kubernetes.SubstrateConfig, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "value not found for") + require.Contains(t, err.Error(), "GIT_INITIALIZER_WINDOWS_IMAGE") + }, + }, + { + name: "GIT_INITIALIZER_WINDOWS_IMAGE_PULL_POLICY not set", + setup: func() { + t.Setenv( + "GIT_INITIALIZER_WINDOWS_IMAGE", + testGitInitializerWindowsImage, + ) + }, + assertions: func(_ kubernetes.SubstrateConfig, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "value not found for") + require.Contains( + t, + err.Error(), + "GIT_INITIALIZER_WINDOWS_IMAGE_PULL_POLICY", + ) + }, + }, + { + name: "DEFAULT_WORKER_IMAGE not set", + setup: func() { + t.Setenv( + "GIT_INITIALIZER_WINDOWS_IMAGE_PULL_POLICY", + string(testGitInitializerWindowsImagePullPolicy), + ) + }, assertions: func(_ kubernetes.SubstrateConfig, err error) { require.Error(t, err) require.Contains(t, err.Error(), "value not found for") @@ -266,6 +303,16 @@ func TestSubstrateConfig(t *testing.T) { testGitInitializerImagePullPolicy, config.GitInitializerImagePullPolicy, ) + require.Equal( + t, + testGitInitializerWindowsImage, + config.GitInitializerWindowsImage, + ) + require.Equal( + t, + testGitInitializerWindowsImagePullPolicy, + config.GitInitializerWindowsImagePullPolicy, + ) require.Equal(t, testDefaultWorkerImage, config.DefaultWorkerImage) require.Equal( t, diff --git a/v2/apiserver/internal/api/jobs.go b/v2/apiserver/internal/api/jobs.go index 72446b115..d7a6416a1 100644 --- a/v2/apiserver/internal/api/jobs.go +++ b/v2/apiserver/internal/api/jobs.go @@ -14,6 +14,16 @@ import ( // JobKind represents the canonical Job kind string const JobKind = "Job" +// OSFamily represents a type of operating system. +type OSFamily string + +const ( + // OSFamilyLinux represents a Linux-based OS. + OSFamilyLinux OSFamily = "linux" + // OSFamilyWindows represents a Windows-based OS. + OSFamilyWindows OSFamily = "windows" +) + // JobPhase represents where a Job is within its lifecycle. type JobPhase string @@ -216,7 +226,7 @@ type JobHost struct { // OS specifies which "family" of operating system is required on a substrate // node to host a Job. Valid values are "linux" and "windows". When empty, // Brigade assumes "linux". - OS string `json:"os,omitempty" bson:"os,omitempty"` + OS OSFamily `json:"os,omitempty" bson:"os,omitempty"` // NodeSelector specifies labels that must be present on the substrate node to // host a Job. This provides an opaque mechanism for communicating Job needs // such as specific hardware like an SSD or GPU. diff --git a/v2/apiserver/internal/api/kubernetes/substrate.go b/v2/apiserver/internal/api/kubernetes/substrate.go index 12112bd31..484a741bb 100644 --- a/v2/apiserver/internal/api/kubernetes/substrate.go +++ b/v2/apiserver/internal/api/kubernetes/substrate.go @@ -50,13 +50,20 @@ type SubstrateConfig struct { // this information whenever it needs to tell another component where to find // the API server. APIAddress string - // GitInitializerImage is the name of the OCI image that will be used (when - // applicable) for the git initializer. The expected format is + // GitInitializerImage is the name of the Linux-based OCI image that will be + // used (when applicable) for the git initializer. The expected format is // [REGISTRY/][ORG/]IMAGE_NAME[:TAG]. GitInitializerImage string // GitInitializerImagePullPolicy is the ImagePullPolicy that will be used - // (when applicable) for the git initializer. + // (when applicable) for the Linux-based git initializer. GitInitializerImagePullPolicy api.ImagePullPolicy + // GitInitializerWindowsImage is the name of the Windows-based OCI image that + // will be used (when applicable) for the git initializer. The expected format + // is [REGISTRY/][ORG/]IMAGE_NAME[:TAG]. + GitInitializerWindowsImage string + // GitInitializerWindowsImagePullPolicy is the ImagePullPolicy that will be + // used (when applicable) for the Windows-based git initializer. + GitInitializerWindowsImagePullPolicy api.ImagePullPolicy // DefaultWorkerImage is the name of the OCI image that will be used for the // Worker pod's container[0] if none is specified in a Project's // configuration. The expected format is [REGISTRY/][ORG/]IMAGE_NAME[:TAG]. @@ -1167,13 +1174,18 @@ func (s *substrate) createJobPod( if useSource && event.Worker.Spec.Git != nil && event.Worker.Spec.Git.CloneURL != "" { + gitInitializerImage := s.config.GitInitializerImage + gitInitializerImagePullPolicy := s.config.GitInitializerImagePullPolicy + if jobSpec.Host != nil && jobSpec.Host.OS == api.OSFamilyWindows { + gitInitializerImage = s.config.GitInitializerWindowsImage + gitInitializerImagePullPolicy = + s.config.GitInitializerWindowsImagePullPolicy + } initContainers = []corev1.Container{ { - Name: "vcs", - Image: s.config.GitInitializerImage, - ImagePullPolicy: corev1.PullPolicy( - s.config.GitInitializerImagePullPolicy, - ), + Name: "vcs", + Image: gitInitializerImage, + ImagePullPolicy: corev1.PullPolicy(gitInitializerImagePullPolicy), VolumeMounts: []corev1.VolumeMount{ { Name: myk8s.LabelKeyEvent, @@ -1238,23 +1250,36 @@ func (s *substrate) createJobPod( }, } + jobPod.Spec.NodeSelector = map[string]string{} + if jobSpec.Host != nil && jobSpec.Host.OS == api.OSFamilyWindows { + jobPod.Spec.NodeSelector[corev1.LabelOSStable] = "windows" + } if s.config.NodeSelectorKey != "" && s.config.NodeSelectorValue != "" { - jobPod.Spec.NodeSelector = map[string]string{ - s.config.NodeSelectorKey: s.config.NodeSelectorValue, - } + jobPod.Spec.NodeSelector[s.config.NodeSelectorKey] = + s.config.NodeSelectorValue } - if s.config.TolerationKey != "" { + jobPod.Spec.Tolerations = []corev1.Toleration{} + if jobSpec.Host != nil && jobSpec.Host.OS == api.OSFamilyWindows { jobPod.Spec.Tolerations = []corev1.Toleration{ { - Key: s.config.TolerationKey, - Operator: corev1.TolerationOpExists, + Key: "os", + Operator: corev1.TolerationOpEqual, + Value: "windows", + Effect: corev1.TaintEffectNoSchedule, }, } + } + if s.config.TolerationKey != "" { + toleration := corev1.Toleration{ + Key: s.config.TolerationKey, + Operator: corev1.TolerationOpExists, + } if s.config.TolerationValue != "" { - jobPod.Spec.Tolerations[0].Value = s.config.TolerationValue - jobPod.Spec.Tolerations[0].Operator = corev1.TolerationOpEqual + toleration.Value = s.config.TolerationValue + toleration.Operator = corev1.TolerationOpEqual } + jobPod.Spec.Tolerations = append(jobPod.Spec.Tolerations, toleration) } podClient := s.kubeClient.CoreV1().Pods(project.Kubernetes.Namespace) diff --git a/v2/apiserver/internal/api/kubernetes/substrate_test.go b/v2/apiserver/internal/api/kubernetes/substrate_test.go index b20a54138..5736cac51 100644 --- a/v2/apiserver/internal/api/kubernetes/substrate_test.go +++ b/v2/apiserver/internal/api/kubernetes/substrate_test.go @@ -1397,6 +1397,12 @@ func TestSubstrateCreateJobSecret(t *testing.T) { } func TestSubstrateCreateJobPod(t *testing.T) { + testSubstrateConfig := SubstrateConfig{ + GitInitializerImage: "brigadecore/brigade2-git-initializer:v2.0.0", // nolint: lll + GitInitializerImagePullPolicy: "IfNotPresent", + GitInitializerWindowsImage: "brigadecore/brigade2-git-initializer-windows:v2.0.0", // nolint: lll + GitInitializerWindowsImagePullPolicy: "IfNotPresent", + } testProject := api.Project{ Kubernetes: &api.KubernetesDetails{ Namespace: "foo", @@ -1447,6 +1453,7 @@ func TestSubstrateCreateJobPod(t *testing.T) { testCases := []struct { name string setup func() *substrate + jobSpec func() api.JobSpec assertions func(kubernetes.Interface, error) }{ { @@ -1467,9 +1474,13 @@ func TestSubstrateCreateJobPod(t *testing.T) { ) require.NoError(t, err) return &substrate{ + config: testSubstrateConfig, kubeClient: kubeClient, } }, + jobSpec: func() api.JobSpec { + return testJobSpec + }, assertions: func(_ kubernetes.Interface, err error) { require.Error(t, err) require.Contains(t, err.Error(), "error creating pod for event") @@ -1479,9 +1490,13 @@ func TestSubstrateCreateJobPod(t *testing.T) { name: "success", setup: func() *substrate { return &substrate{ + config: testSubstrateConfig, kubeClient: fake.NewSimpleClientset(), } }, + jobSpec: func() api.JobSpec { + return testJobSpec + }, assertions: func(kubeClient kubernetes.Interface, err error) { require.NoError(t, err) pod, err := kubeClient.CoreV1().Pods( @@ -1502,6 +1517,16 @@ func TestSubstrateCreateJobPod(t *testing.T) { // Init container: require.Len(t, pod.Spec.InitContainers, 1) require.Equal(t, "vcs", pod.Spec.InitContainers[0].Name) + require.Equal( + t, + testSubstrateConfig.GitInitializerImage, + pod.Spec.InitContainers[0].Image, + ) + require.Equal( + t, + corev1.PullPolicy(testSubstrateConfig.GitInitializerImagePullPolicy), + pod.Spec.InitContainers[0].ImagePullPolicy, + ) require.Len(t, pod.Spec.InitContainers[0].VolumeMounts, 2) require.Equal( t, @@ -1545,6 +1570,49 @@ func TestSubstrateCreateJobPod(t *testing.T) { // ) }, }, + { + name: "success with windows", + setup: func() *substrate { + return &substrate{ + config: testSubstrateConfig, + kubeClient: fake.NewSimpleClientset(), + } + }, + jobSpec: func() api.JobSpec { + jobSpecCopy := testJobSpec + jobSpecCopy.Host = &api.JobHost{ + OS: api.OSFamilyWindows, + } + return jobSpecCopy + }, + assertions: func(kubeClient kubernetes.Interface, err error) { + require.NoError(t, err) + pod, err := kubeClient.CoreV1().Pods( + testProject.Kubernetes.Namespace, + ).Get( + context.Background(), + myk8s.JobPodName(testEvent.ID, testJobName), + metav1.GetOptions{}, + ) + require.NoError(t, err) + require.NotNil(t, pod) + // These should be the only real differences from the previous test case + // + // Make sure the pod is assigned to a Windows node + require.Equal(t, "windows", pod.Spec.NodeSelector[corev1.LabelOSStable]) + // Make sure we use the Windows variant of the git initializer image + require.Equal( + t, + testSubstrateConfig.GitInitializerWindowsImage, + pod.Spec.InitContainers[0].Image, + ) + require.Equal( + t, + corev1.PullPolicy(testSubstrateConfig.GitInitializerWindowsImagePullPolicy), // nolint: lll + pod.Spec.InitContainers[0].ImagePullPolicy, + ) + }, + }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { @@ -1554,7 +1622,7 @@ func TestSubstrateCreateJobPod(t *testing.T) { testProject, testEvent, testJobName, - testJobSpec, + testCase.jobSpec(), ) testCase.assertions(substrate.kubeClient, err) }) @@ -1597,8 +1665,8 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, workerPod) - require.Nil(t, workerPod.Spec.NodeSelector) - require.Nil(t, workerPod.Spec.Tolerations) + require.Empty(t, workerPod.Spec.NodeSelector) + require.Empty(t, workerPod.Spec.Tolerations) jobPod, err := kubeClient.CoreV1().Pods( testProject.Kubernetes.Namespace, @@ -1609,8 +1677,8 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, jobPod) - require.Nil(t, jobPod.Spec.NodeSelector) - require.Nil(t, jobPod.Spec.Tolerations) + require.Empty(t, jobPod.Spec.NodeSelector) + require.Empty(t, jobPod.Spec.Tolerations) }, }, { @@ -1633,8 +1701,8 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, workerPod) - require.Nil(t, workerPod.Spec.NodeSelector) - require.Nil(t, workerPod.Spec.Tolerations) + require.Empty(t, workerPod.Spec.NodeSelector) + require.Empty(t, workerPod.Spec.Tolerations) jobPod, err := kubeClient.CoreV1().Pods( testProject.Kubernetes.Namespace, @@ -1645,8 +1713,8 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, jobPod) - require.Nil(t, jobPod.Spec.NodeSelector) - require.Nil(t, jobPod.Spec.Tolerations) + require.Empty(t, jobPod.Spec.NodeSelector) + require.Empty(t, jobPod.Spec.Tolerations) }, }, { @@ -1671,7 +1739,7 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { require.NoError(t, err) require.NotNil(t, workerPod) require.Equal(t, "bar", workerPod.Spec.NodeSelector["foo"]) - require.Nil(t, workerPod.Spec.Tolerations) + require.Empty(t, workerPod.Spec.Tolerations) jobPod, err := kubeClient.CoreV1().Pods( testProject.Kubernetes.Namespace, @@ -1683,7 +1751,7 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { require.NoError(t, err) require.NotNil(t, jobPod) require.Equal(t, "bar", jobPod.Spec.NodeSelector["foo"]) - require.Nil(t, jobPod.Spec.Tolerations) + require.Empty(t, jobPod.Spec.Tolerations) }, }, { @@ -1706,7 +1774,7 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, workerPod) - require.Nil(t, workerPod.Spec.NodeSelector) + require.Empty(t, workerPod.Spec.NodeSelector) require.Equal(t, corev1.Toleration{ Key: "foo", Operator: corev1.TolerationOpExists, @@ -1721,7 +1789,7 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, jobPod) - require.Nil(t, jobPod.Spec.NodeSelector) + require.Empty(t, jobPod.Spec.NodeSelector) require.Equal(t, corev1.Toleration{ Key: "foo", Operator: corev1.TolerationOpExists, @@ -1749,7 +1817,7 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, workerPod) - require.Nil(t, workerPod.Spec.NodeSelector) + require.Empty(t, workerPod.Spec.NodeSelector) require.Equal(t, corev1.Toleration{ Key: "foo", Value: "bar", @@ -1765,7 +1833,7 @@ func TestSubstrateCreatePodWithNodeSelectorAndToleration(t *testing.T) { ) require.NoError(t, err) require.NotNil(t, jobPod) - require.Nil(t, jobPod.Spec.NodeSelector) + require.Empty(t, jobPod.Spec.NodeSelector) require.Equal(t, corev1.Toleration{ Key: "foo", Value: "bar",