Skip to content

Commit

Permalink
fix: don't use generateName when creating jobs (#48)
Browse files Browse the repository at this point in the history
  • Loading branch information
erikgb authored Jan 16, 2023
1 parent 490ec81 commit d7c8185
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 38 deletions.
1 change: 0 additions & 1 deletion api/v1alpha1/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package v1alpha1
const (
LabelK8sAppName = "app.kubernetes.io/name"
LabelK8SAppManagedBy = "app.kubernetes.io/managed-by"
LabelStatnettControllerHash = "controller.statnett.no/hash"
LabelStatnettControllerNamespace = "controller.statnett.no/namespace"
LabelStatnettControllerUID = "controller.statnett.no/uid"

Expand Down
52 changes: 21 additions & 31 deletions controllers/containerimagescan_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
stasv1alpha1 "github.com/statnett/image-scanner-operator/api/v1alpha1"
"github.com/statnett/image-scanner-operator/internal/controller"
staserrors "github.com/statnett/image-scanner-operator/internal/errors"
"github.com/statnett/image-scanner-operator/internal/hash"
"github.com/statnett/image-scanner-operator/internal/trivy"
"github.com/statnett/image-scanner-operator/pkg/operator"
)
Expand Down Expand Up @@ -75,33 +74,37 @@ func (r *ContainerImageScanReconciler) SetupWithManager(mgr ctrl.Manager) error
func (r *ContainerImageScanReconciler) reconcile(ctx context.Context, cis *stasv1alpha1.ContainerImageScan) error {
cleanCis := cis.DeepCopy()

scanJobs := &batchv1.JobList{}
if err := r.listScanJobs(ctx, cis, scanJobs); err != nil {
scanJob, err := r.newScanJob(ctx, cis)
if err != nil {
return err
}
// Don't create duplicate jobs
if len(scanJobs.Items) == 0 {
scanJob, err := r.createScanJob(ctx, cis)
if err != nil {
return err
}

condition := metav1.Condition{
Type: string(kstatus.ConditionReconciling),
Status: metav1.ConditionTrue,
Reason: "ScanJobCreated",
Message: fmt.Sprintf("Job '%s' created to scan image.", scanJob.Name),
// Jobs are highly immutable, so not attempting to update
err = r.Create(ctx, scanJob)
if err != nil {
if apierrors.IsAlreadyExists(err) {
// Don't create duplicate jobs
return nil
}
meta.SetStatusCondition(&cis.Status.Conditions, condition)
meta.RemoveStatusCondition(&cis.Status.Conditions, string(kstatus.ConditionStalled))

return err
}

condition := metav1.Condition{
Type: string(kstatus.ConditionReconciling),
Status: metav1.ConditionTrue,
Reason: "ScanJobCreated",
Message: fmt.Sprintf("Job '%s' created to scan image.", scanJob.Name),
}
meta.SetStatusCondition(&cis.Status.Conditions, condition)
meta.RemoveStatusCondition(&cis.Status.Conditions, string(kstatus.ConditionStalled))

cis.Status.ObservedGeneration = cis.Generation

return r.Status().Patch(ctx, cis, client.MergeFrom(cleanCis))
}

func (r *ContainerImageScanReconciler) createScanJob(ctx context.Context, cis *stasv1alpha1.ContainerImageScan) (*batchv1.Job, error) {
func (r *ContainerImageScanReconciler) newScanJob(ctx context.Context, cis *stasv1alpha1.ContainerImageScan) (*batchv1.Job, error) {
var nodeNames []string

for _, or := range cis.OwnerReferences {
Expand All @@ -127,18 +130,5 @@ func (r *ContainerImageScanReconciler) createScanJob(ctx context.Context, cis *s
return nil, err
}

return job, r.Create(ctx, job)
}

func (r *ContainerImageScanReconciler) listScanJobs(ctx context.Context, cis *stasv1alpha1.ContainerImageScan, jobs *batchv1.JobList) error {
matchLabels := map[string]string{
stasv1alpha1.LabelStatnettControllerUID: string(cis.UID),
stasv1alpha1.LabelStatnettControllerHash: hash.NewString(cis.Spec),
}
listOps := []client.ListOption{
client.InNamespace(r.ScanJobNamespace),
client.MatchingLabels(matchLabels),
}

return r.List(ctx, jobs, listOps...)
return job, nil
}
1 change: 0 additions & 1 deletion controllers/containerimagescan_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ var _ = Describe("ContainerImageScan controller", func() {
normalizeUntestableScanJobFields := func(job *batchv1.Job) *batchv1.Job {
job.APIVersion = "batch/v1"
job.Kind = "Job"
job.Name = ""
job.UID = ""
job.ResourceVersion = ""
job.CreationTimestamp = metav1.Time{}
Expand Down
3 changes: 1 addition & 2 deletions controllers/testdata/scan-job/expected-scan-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ kind: Job
metadata:
annotations:
batch.kubernetes.io/job-tracking: ""
generateName: echo-6bdfc76c56-8ae43
generation: 1
labels:
app.kubernetes.io/managed-by: image-scanner
app.kubernetes.io/name: trivy
controller.statnett.no/hash: 81f104762422c0137dee1c58e2c4d454
controller.statnett.no/namespace: replica-set
controller.statnett.no/uid: <CIS-UID>
namespace: image-scanner-jobs
name: echo-6bdfc76c56-8ae43-8731b
spec:
activeDeadlineSeconds: 3600
backoffLimit: 3
Expand Down
3 changes: 2 additions & 1 deletion controllers/workload_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand All @@ -31,7 +32,7 @@ import (

const (
ImageShortSHALength = 5
KubernetesNameMaxLength = 253
KubernetesNameMaxLength = validation.DNS1123SubdomainMaxLength
)

var noEventsEventHandler = handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {
Expand Down
22 changes: 20 additions & 2 deletions internal/trivy/scan_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package trivy

import (
"embed"
"fmt"
"strings"
"time"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/utils/pointer"

stasv1alpha1 "github.com/statnett/image-scanner-operator/api/v1alpha1"
Expand All @@ -19,6 +21,8 @@ const (
FsScanSharedVolumeMountPath = "/var/run/image-scanner"
FsScanSharedVolumeName = "image-scanner"
FsScanTrivyBinaryPath = FsScanSharedVolumeMountPath + "/trivy"
JobNameSpecHashPartLength = 5
KubernetesJobNameMaxLength = validation.DNS1123LabelMaxLength
ScanJobContainerName = "scan-image"
ScanJobTimeout = 1 * time.Hour
TempVolumeName = "tmp"
Expand Down Expand Up @@ -63,18 +67,32 @@ func (f *filesystemScanJobBuilder) ForCIS(cis *stasv1alpha1.ContainerImageScan)
}

job.Namespace = f.ScanJobNamespace
job.GenerateName = cis.Name
job.Name = scanJobName(cis)
job.Labels = map[string]string{
stasv1alpha1.LabelK8sAppName: stasv1alpha1.AppNameTrivy,
stasv1alpha1.LabelK8SAppManagedBy: stasv1alpha1.AppNameImageScanner,
stasv1alpha1.LabelStatnettControllerNamespace: cis.Namespace,
stasv1alpha1.LabelStatnettControllerUID: string(cis.UID),
stasv1alpha1.LabelStatnettControllerHash: hash.NewString(cis.Spec),
}

return job, nil
}

func scanJobName(cis *stasv1alpha1.ContainerImageScan) string {
hashPart := hash.NewString(cis.Spec, cis.Namespace)[0:JobNameSpecHashPartLength]
nameFn := func(cisName string) string {
return fmt.Sprintf("%s-%s", cisName, hashPart)
}

name := nameFn(cis.Name)
if len(name) > KubernetesJobNameMaxLength {
shortenCISName := cis.Name[0 : len(cis.Name)-(len(name)-KubernetesJobNameMaxLength)]
name = nameFn(shortenCISName)
}

return name
}

func (f *filesystemScanJobBuilder) newImageScanJob(spec stasv1alpha1.ContainerImageScanSpec) (*batchv1.Job, error) {
job := &batchv1.Job{}

Expand Down

0 comments on commit d7c8185

Please sign in to comment.