Skip to content

Commit

Permalink
Restart batch with active deployment (#897)
Browse files Browse the repository at this point in the history
* Added secret type label

* Added secret type label helper method

* Removed returned error from label requirement method

* Added GetRadixBatchDescendantsSelector

* Validated job payload secrets on sync

* Set version

* Fix unit test

* Trigger build
  • Loading branch information
satr authored Jul 27, 2023
1 parent cfe961a commit d18a4a7
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 10 deletions.
4 changes: 2 additions & 2 deletions charts/radix-operator/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v2
name: radix-operator
version: 1.19.5
appVersion: 1.39.5
version: 1.19.6
appVersion: 1.39.6
kubeVersion: ">=1.24.0"
description: Radix Operator
keywords:
Expand Down
26 changes: 26 additions & 0 deletions pkg/apis/batch/kubejob.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package batch

import (
"context"
"fmt"

"github.com/equinor/radix-common/utils/numbers"
"github.com/equinor/radix-common/utils/pointers"
Expand All @@ -16,6 +17,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/util/retry"
)

Expand All @@ -38,6 +40,10 @@ func (s *syncer) reconcileKubeJob(batchJob *radixv1.RadixBatchJob, rd *radixv1.R
slice.Any(existingJobs, func(job *batchv1.Job) bool { return isResourceLabeledWithBatchJobName(batchJob.Name, job) })) {
return nil
}
err = s.validatePayloadSecretReference(batchJob, jobComponent)
if err != nil {
return err
}
job, err := s.buildJob(batchJob, jobComponent, rd)
if err != nil {
return err
Expand All @@ -48,6 +54,26 @@ func (s *syncer) reconcileKubeJob(batchJob *radixv1.RadixBatchJob, rd *radixv1.R
})
}

func (s *syncer) validatePayloadSecretReference(batchJob *radixv1.RadixBatchJob, jobComponent *radixv1.RadixDeployJobComponent) error {
if batchJob.PayloadSecretRef == nil {
return nil
}
payloadSecret, err := s.kubeclient.CoreV1().Secrets(s.batch.GetNamespace()).Get(context.Background(), batchJob.PayloadSecretRef.Name, metav1.GetOptions{})
if err != nil {
return err
}
if !radixlabels.GetRadixBatchDescendantsSelector(jobComponent.GetName()).Matches(labels.Set(payloadSecret.GetLabels())) {
return fmt.Errorf("secret %s, referenced in the job %s of the batch %s is not valid payload secret", batchJob.PayloadSecretRef.Name, batchJob.Name, s.batch.GetName())
}
if payloadSecret.Data == nil || len(payloadSecret.Data) == 0 {
return fmt.Errorf("payload secret %s, in the job %s of the batch %s is empty", batchJob.PayloadSecretRef.Name, batchJob.Name, s.batch.GetName())
}
if _, ok := payloadSecret.Data[batchJob.PayloadSecretRef.Key]; !ok {
return fmt.Errorf("payload secret %s, in the job %s of the batch %s has no entry %s for the job", batchJob.PayloadSecretRef.Name, batchJob.Name, s.batch.GetName(), batchJob.PayloadSecretRef.Key)
}
return nil
}

func (s *syncer) handleJobToRestart(batchJob *radixv1.RadixBatchJob, existingJobs []*batchv1.Job) (bool, error) {
jobStatusIdx := slice.FindIndex(s.batch.Status.JobStatuses, func(jobStatus radixv1.RadixBatchJobStatus) bool {
return jobStatus.Name == batchJob.Name
Expand Down
15 changes: 14 additions & 1 deletion pkg/apis/batch/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
radixv1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
"github.com/equinor/radix-operator/pkg/apis/securitycontext"
"github.com/equinor/radix-operator/pkg/apis/utils"
radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels"
fakeradix "github.com/equinor/radix-operator/pkg/client/clientset/versioned/fake"
prometheusfake "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned/fake"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -746,7 +747,19 @@ func (s *syncerTestSuite) Test_JobWithPayload() {
},
},
}
batch, err := s.radixClient.RadixV1().RadixBatches(namespace).Create(context.Background(), batch, metav1.CreateOptions{})
secret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Labels: radixlabels.Merge(radixlabels.ForJobScheduleJobType(),
radixlabels.ForApplicationName(appName),
radixlabels.ForComponentName(componentName), radixlabels.ForBatchName(batchName)),
},
Data: map[string][]byte{secretKey: []byte("any-payload")},
}

_, err := s.kubeClient.CoreV1().Secrets(namespace).Create(context.Background(), &secret, metav1.CreateOptions{})
s.Require().NoError(err)
batch, err = s.radixClient.RadixV1().RadixBatches(namespace).Create(context.Background(), batch, metav1.CreateOptions{})
s.Require().NoError(err)
_, err = s.radixClient.RadixV1().RadixDeployments(namespace).Create(context.Background(), rd, metav1.CreateOptions{})
s.Require().NoError(err)
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/kube/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const (
RadixGpuCountLabel = "radix-node-gpu-count"
RadixNamespace = "radix-namespace"
RadixConfigMapTypeLabel = "radix-config-map-type"
RadixSecretTypeLabel = "radix-secret-type"
RadixSecretRefTypeLabel = "radix-secret-ref-type"
RadixSecretRefNameLabel = "radix-secret-ref-name"
RadixUserDefinedNetworkPolicyLabel = "is-user-defined"
Expand All @@ -84,6 +85,13 @@ const (
RadixBatchTypeBatch RadixBatchType = "batch"
)

// RadixSecretType defines value for use with label RadixSecretTypeLabel
type RadixSecretType string

const (
RadixSecretJobPayload RadixSecretType = "scheduler-job-payload"
)

// RadixConfigMapType Purpose of ConfigMap
type RadixConfigMapType string

Expand Down
23 changes: 20 additions & 3 deletions pkg/apis/utils/labels/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,24 @@ func forAzureWorkloadUseIdentity() kubelabels.Set {
}
}

// RequirementRadixBatchNameLabelExists returns a requirement that the label RadixBatchNameLabel exists
func RequirementRadixBatchNameLabelExists() (*kubelabels.Requirement, error) {
return kubelabels.NewRequirement(kube.RadixBatchNameLabel, selection.Exists, []string{})
// GetRadixBatchDescendantsSelector returns selector for radix batch descendants - jobs, secrets, etc.
func GetRadixBatchDescendantsSelector(componentName string) kubelabels.Selector {
return kubelabels.SelectorFromSet(Merge(ForJobScheduleJobType(), ForComponentName(componentName))).
Add(*requirementRadixBatchNameLabelExists())
}

func requirementRadixBatchNameLabelExists() *kubelabels.Requirement {
requirement, err := kubelabels.NewRequirement(kube.RadixBatchNameLabel, selection.Exists, []string{})
if err != nil {
panic(err)
}
return requirement
}

// ForRadixSecretType returns labels describing the radix secret type,
// e.g. "radix-secret-type": "scheduler-job-payload"
func ForRadixSecretType(secretType kube.RadixSecretType) kubelabels.Set {
return kubelabels.Set{
kube.RadixSecretTypeLabel: string(secretType),
}
}
42 changes: 38 additions & 4 deletions pkg/apis/utils/labels/labels_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package labels

import (
"k8s.io/apimachinery/pkg/selection"
"testing"

"github.com/equinor/radix-operator/pkg/apis/kube"
Expand Down Expand Up @@ -108,7 +107,42 @@ func Test_ForJobScheduleJobType(t *testing.T) {
}

func Test_RequirementRadixBatchNameLabelExists(t *testing.T) {
actual, _ := RequirementRadixBatchNameLabelExists()
expected, _ := kubelabels.NewRequirement(kube.RadixBatchNameLabel, selection.Exists, []string{})
assert.Equal(t, expected, actual)
actual := requirementRadixBatchNameLabelExists()
expected := kubelabels.Set{kube.RadixBatchNameLabel: "anyname"}
assert.True(t, actual.Matches(expected))
}

func TestGetRadixBatchDescendantsSelector(t *testing.T) {
type args struct {
componentName string
labels kubelabels.Set
}
tests := []struct {
name string
args args
want bool
}{
{name: "No labels", args: args{componentName: "anycomponentname", labels: kubelabels.Set{}}, want: false},
{name: "Wrong component name", args: args{componentName: "different-comp",
labels: Merge(ForComponentName("comp1"), ForJobScheduleJobType(), ForBatchName("somebatch"))},
want: false},
{name: "No batch name", args: args{componentName: "comp1",
labels: Merge(ForComponentName("comp1"), ForJobScheduleJobType())},
want: false},
{name: "No job type job schedule", args: args{componentName: "different-comp",
labels: Merge(ForComponentName("comp1"), ForBatchName("somebatch"))},
want: false},
{name: "Wrong job type job schedule", args: args{componentName: "different-comp",
labels: Merge(ForComponentName("comp1"), ForBatchName("somebatch"), kubelabels.Set{
kube.RadixJobTypeLabel: "other-type"})},
want: false},
{name: "Correct component name and all labels exist", args: args{componentName: "comp1",
labels: Merge(ForComponentName("comp1"), ForJobScheduleJobType(), ForBatchName("somebatch"))},
want: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, GetRadixBatchDescendantsSelector(tt.args.componentName).Matches(tt.args.labels), "GetRadixBatchDescendantsSelector(%v)", tt.args.componentName)
})
}
}

0 comments on commit d18a4a7

Please sign in to comment.