Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Commit

Permalink
Merge aa1b50c into 491e84a
Browse files Browse the repository at this point in the history
  • Loading branch information
Mario Manno committed Apr 2, 2020
2 parents 491e84a + aa1b50c commit b4e40a6
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 46 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
- stage: unit
services: []
before_script:
- curl -LO https://github.com/dominikh/go-tools/releases/download/2019.2.3/staticcheck_linux_amd64.tar.gz
- curl -LO https://github.com/dominikh/go-tools/releases/download/2020.1.3/staticcheck_linux_amd64.tar.gz
- tar xfz staticcheck_linux_amd64.tar.gz --strip-component 1 -C $GOPATH/bin staticcheck/staticcheck
- go get -u golang.org/x/lint/golint
script: make lint
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module code.cloudfoundry.org/quarks-job

require (
code.cloudfoundry.org/quarks-utils v0.0.0-20200319162833-2e0d9adb7eb6
code.cloudfoundry.org/quarks-utils v0.0.0-20200331122601-bc0838ffea60
github.com/onsi/ginkgo v1.10.2
github.com/onsi/gomega v1.6.0
github.com/pkg/errors v0.8.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU=
code.cloudfoundry.org/quarks-utils v0.0.0-20200225114218-378cd73b8745 h1:BpXjjRdTSIR52m3B0ZLllLRpCAuuB/SsFk1c9rA7FWI=
code.cloudfoundry.org/quarks-utils v0.0.0-20200225114218-378cd73b8745/go.mod h1:d2OaSM1qVE/7Zo1imovL7CZCOAShFePFMI3jlpMcp14=
code.cloudfoundry.org/quarks-utils v0.0.0-20200331122601-bc0838ffea60 h1:1FNLU6zVwDXkLXvJS7RvSScxNpO01NfyiR6Za9Un5Cg=
code.cloudfoundry.org/quarks-utils v0.0.0-20200331122601-bc0838ffea60/go.mod h1:d2OaSM1qVE/7Zo1imovL7CZCOAShFePFMI3jlpMcp14=
github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+BFDTQ8rEZB0s4Yfa6bI=
github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0=
Expand Down
5 changes: 2 additions & 3 deletions integration/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

. "github.com/onsi/ginkgo"
"github.com/onsi/gomega"
. "github.com/onsi/gomega"

"k8s.io/client-go/rest"
Expand Down Expand Up @@ -53,7 +52,7 @@ var _ = BeforeEach(func() {

err := env.SetupClientsets()
if err != nil {
gomega.Expect(err).NotTo(gomega.HaveOccurred())
Expect(err).NotTo(HaveOccurred())
}

err = env.SetupNamespace()
Expand All @@ -69,7 +68,7 @@ var _ = BeforeEach(func() {

env.Stop, err = env.StartOperator()
if err != nil {
gomega.Expect(err).NotTo(gomega.HaveOccurred())
Expect(err).NotTo(HaveOccurred())
}
})

Expand Down
6 changes: 6 additions & 0 deletions pkg/kube/apis/quarksjob/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type SecretOptions struct {
PersistenceMethod PersistenceMethod `json:"persistencemethod,omitempty"`
}

// FanOutName returns the name of the secret for PersistenceMethod 'fan-out'
func (so SecretOptions) FanOutName(key string) string {
return so.Name + "-" + key
}
Expand Down Expand Up @@ -146,6 +147,11 @@ func (q *QuarksJob) IsAutoErrand() bool {
return q.Spec.Trigger.Strategy == TriggerOnce || q.Spec.Trigger.Strategy == TriggerDone
}

// GetNamespacedName returns the resource name with its namespace
func (q *QuarksJob) GetNamespacedName() string {
return fmt.Sprintf("%s/%s", q.Namespace, q.Name)
}

// NewFileToSecret returns a FilesToSecrets with just one mapping
func NewFileToSecret(fileName string, secretName string, versioned bool) FilesToSecrets {
return FilesToSecrets{
Expand Down
12 changes: 7 additions & 5 deletions pkg/kube/controllers/quarksjob/errand_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

qjv1a1 "code.cloudfoundry.org/quarks-job/pkg/kube/apis/quarksjob/v1alpha1"
"code.cloudfoundry.org/quarks-job/pkg/kube/util/reference"
"code.cloudfoundry.org/quarks-job/pkg/kube/util/config"
"code.cloudfoundry.org/quarks-job/pkg/kube/util/reference"
"code.cloudfoundry.org/quarks-utils/pkg/ctxlog"
"code.cloudfoundry.org/quarks-utils/pkg/names"
vss "code.cloudfoundry.org/quarks-utils/pkg/versionedsecretstore"
Expand Down Expand Up @@ -50,7 +50,8 @@ func AddErrand(ctx context.Context, config *config.Config, mgr manager.Manager)
if shouldProcessEvent {
ctxlog.NewPredicateEvent(qJob).Debug(
ctx, e.Meta, qjv1a1.QuarksJobResourceName,
fmt.Sprintf("Create predicate passed for '%s', existing quarksJob spec.Trigger.Strategy matches the values 'now' or 'once'",
fmt.Sprintf("Create predicate passed for '%s/%s', existing quarksJob spec.Trigger.Strategy matches the values 'now' or 'once'",
e.Meta.GetNamespace(),
e.Meta.GetName()),
)
}
Expand All @@ -76,7 +77,8 @@ func AddErrand(ctx context.Context, config *config.Config, mgr manager.Manager)
if shouldProcessEvent {
ctxlog.NewPredicateEvent(o).Debug(
ctx, e.MetaNew, qjv1a1.QuarksJobResourceName,
fmt.Sprintf("Update predicate passed for '%s', a change in it´s referenced secrets have been detected",
fmt.Sprintf("Update predicate passed for '%s/%s', a change in it´s referenced secrets have been detected",
e.MetaNew.GetNamespace(),
e.MetaNew.GetName()),
)
}
Expand Down Expand Up @@ -114,7 +116,7 @@ func AddErrand(ctx context.Context, config *config.Config, mgr manager.Manager)

reconciles, err := reference.GetReconciles(ctx, mgr.GetClient(), reference.ReconcileForQuarksJob, cm)
if err != nil {
ctxlog.Errorf(ctx, "Failed to calculate reconciles for config '%s': %v", cm.Name, err)
ctxlog.Errorf(ctx, "Failed to calculate reconciles for config '%s/%s': %v", cm.Namespace, cm.Name, err)
}

for _, reconciliation := range reconciles {
Expand Down Expand Up @@ -162,7 +164,7 @@ func AddErrand(ctx context.Context, config *config.Config, mgr manager.Manager)

reconciles, err := reference.GetReconciles(ctx, mgr.GetClient(), reference.ReconcileForQuarksJob, s)
if err != nil {
ctxlog.Errorf(ctx, "Failed to calculate reconciles for secret '%s': %v", s.Name, err)
ctxlog.Errorf(ctx, "Failed to calculate reconciles for secret '%s/%s': %v", s.Namespace, s.Name, err)
}

for _, reconciliation := range reconciles {
Expand Down
14 changes: 7 additions & 7 deletions pkg/kube/controllers/quarksjob/errand_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,37 +85,37 @@ func (r *ErrandReconciler) Reconcile(request reconcile.Request) (reconcile.Resul
}

if meltdown.NewWindow(r.config.MeltdownDuration, qJob.Status.LastReconcile).Contains(time.Now()) {
ctxlog.WithEvent(qJob, "Meltdown").Debugf(ctx, "Resource '%s' is in meltdown, requeue reconcile after %s", qJob.Name, r.config.MeltdownRequeueAfter)
ctxlog.WithEvent(qJob, "Meltdown").Debugf(ctx, "Resource '%s' is in meltdown, requeue reconcile after %s", request.NamespacedName, r.config.MeltdownRequeueAfter)
return reconcile.Result{RequeueAfter: r.config.MeltdownRequeueAfter}, nil
}

if qJob.Spec.Trigger.Strategy == qjv1a1.TriggerNow {
// Set Strategy back to manual for errand jobs.
qJob.Spec.Trigger.Strategy = qjv1a1.TriggerManual
if err := r.client.Update(ctx, qJob); err != nil {
return reconcile.Result{}, ctxlog.WithEvent(qJob, "UpdateError").Errorf(ctx, "Failed to revert to 'trigger.strategy=manual' on job '%s': %s", qJob.Name, err)
return reconcile.Result{}, ctxlog.WithEvent(qJob, "UpdateError").Errorf(ctx, "Failed to revert to 'trigger.strategy=manual' on job '%s': %s", qJob.GetNamespacedName(), err)
}
}

r.injectContainerEnv(&qJob.Spec.Template.Spec.Template.Spec)
if retry, err := r.jobCreator.Create(ctx, *qJob, request.Namespace); err != nil {
return reconcile.Result{}, ctxlog.WithEvent(qJob, "CreateJobError").Errorf(ctx, "Failed to create job '%s': %s", qJob.Name, err)
return reconcile.Result{}, ctxlog.WithEvent(qJob, "CreateJobError").Errorf(ctx, "Failed to create job '%s': %s", qJob.GetNamespacedName(), err)
} else if retry {
ctxlog.Infof(ctx, "Retrying to create job '%s'", qJob.Name)
ctxlog.Infof(ctx, "Retrying to create job '%s'", qJob.GetNamespacedName())
result := reconcile.Result{
Requeue: true,
RequeueAfter: time.Second * 5,
}
return result, nil
}

ctxlog.WithEvent(qJob, "CreateJob").Infof(ctx, "Created errand job for '%s'", qJob.Name)
ctxlog.WithEvent(qJob, "CreateJob").Infof(ctx, "Created errand job for '%s'", qJob.GetNamespacedName())

if qJob.Spec.Trigger.Strategy == qjv1a1.TriggerOnce {
// Traverse Strategy into the final 'done' state.
qJob.Spec.Trigger.Strategy = qjv1a1.TriggerDone
if err := r.client.Update(ctx, qJob); err != nil {
ctxlog.WithEvent(qJob, "UpdateError").Errorf(ctx, "Failed to traverse to 'trigger.strategy=done' on job '%s': %s", qJob.Name, err)
ctxlog.WithEvent(qJob, "UpdateError").Errorf(ctx, "Failed to traverse to 'trigger.strategy=done' on job '%s': %s", qJob.GetNamespacedName(), err)
return reconcile.Result{Requeue: false}, nil
}
}
Expand All @@ -124,7 +124,7 @@ func (r *ErrandReconciler) Reconcile(request reconcile.Request) (reconcile.Resul
qJob.Status.LastReconcile = &now
err := r.client.Status().Update(ctx, qJob)
if err != nil {
ctxlog.WithEvent(qJob, "UpdateError").Errorf(ctx, "Failed to update reconcile timestamp on job '%s' (%v): %s", qJob.Name, qJob.ResourceVersion, err)
ctxlog.WithEvent(qJob, "UpdateError").Errorf(ctx, "Failed to update reconcile timestamp on job '%s' (%v): %s", qJob.GetNamespacedName(), qJob.ResourceVersion, err)
return reconcile.Result{Requeue: false}, nil
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/kube/controllers/quarksjob/errand_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ var _ = Describe("ErrandReconciler", func() {
It("should return and try to requeue", func() {
_, err := act()
Expect(err).To(HaveOccurred())
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Failed to revert to 'trigger.strategy=manual' on job '%s': fake-error", qJobName)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Failed to revert to 'trigger.strategy=manual' on job '/%s': fake-error", qJobName)).Len()).To(Equal(1))
Expect(client.CreateCallCount()).To(Equal(0))
})
})
Expand All @@ -157,7 +157,7 @@ var _ = Describe("ErrandReconciler", func() {

It("should log create error and requeue", func() {
_, err := act()
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Failed to create job '%s': fake-error", qJobName)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Failed to create job '/%s': fake-error", qJobName)).Len()).To(Equal(1))
Expect(err).To(HaveOccurred())
Expect(client.CreateCallCount()).To(Equal(1))
})
Expand All @@ -174,7 +174,7 @@ var _ = Describe("ErrandReconciler", func() {
result, err := act()
Expect(err).NotTo(HaveOccurred())
Expect(result.Requeue).To(BeFalse())
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Skip '%s': already running", qJobName)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Skip '/%s': already running", qJobName)).Len()).To(Equal(1))
Expect(client.CreateCallCount()).To(Equal(1))
})
})
Expand Down Expand Up @@ -386,7 +386,7 @@ var _ = Describe("ErrandReconciler", func() {
result, err := act()
Expect(err).ToNot(HaveOccurred())
Expect(result.Requeue).To(BeTrue())
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Skip create job '%s' due to configMap 'config1' not found", qJobName)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Skip create job '/%s' due to configMap 'config1' not found", qJobName)).Len()).To(Equal(1))

client.GetCalls(func(ctx context.Context, nn types.NamespacedName, obj runtime.Object) error {
switch obj := obj.(type) {
Expand All @@ -406,7 +406,7 @@ var _ = Describe("ErrandReconciler", func() {
result, err = act()
Expect(err).ToNot(HaveOccurred())
Expect(result.Requeue).To(BeTrue())
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Skip create job '%s' due to secret 'secret1' not found", qJobName)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Skip create job '/%s' due to secret 'secret1' not found", qJobName)).Len()).To(Equal(1))
})
})
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/kube/controllers/quarksjob/job_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ func AddJob(ctx context.Context, config *config.Config, mgr manager.Manager) err
if shouldProcessEvent {
ctxlog.NewPredicateEvent(o).Debug(
ctx, e.MetaNew, "batchv1.Job",
fmt.Sprintf("Update predicate passed for '%s', existing batchv1.Job has changed to a final state, either succeeded or failed",
fmt.Sprintf("Update predicate passed for '%s/%s', existing batchv1.Job has changed to a final state, either succeeded or failed",
e.MetaNew.GetNamespace(),
e.MetaNew.GetName()),
)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/kube/controllers/quarksjob/job_creator.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (j jobCreatorImpl) Create(ctx context.Context, qJob qjv1a1.QuarksJob, names
// Create k8s job
name, err := names.JobName(qJob.Name)
if err != nil {
return false, errors.Wrapf(err, "could not generate job name for qJob '%s'", qJob.Name)
return false, errors.Wrapf(err, "could not generate job name for qJob '%s'", qJob.GetNamespacedName())
}

job := &batchv1.Job{
Expand All @@ -146,12 +146,12 @@ func (j jobCreatorImpl) Create(ctx context.Context, qJob qjv1a1.QuarksJob, names
}

if err := j.setOwnerReference(&qJob, job, j.scheme); err != nil {
return false, ctxlog.WithEvent(&qJob, "SetOwnerReferenceError").Errorf(ctx, "failed to set owner reference on job for '%s': %s", qJob.Name, err)
return false, ctxlog.WithEvent(&qJob, "SetOwnerReferenceError").Errorf(ctx, "failed to set owner reference on job for '%s': %s", qJob.GetNamespacedName(), err)
}

if err := j.client.Create(ctx, job); err != nil {
if apierrors.IsAlreadyExists(err) {
ctxlog.WithEvent(&qJob, "AlreadyRunning").Infof(ctx, "Skip '%s': already running", qJob.Name)
ctxlog.WithEvent(&qJob, "AlreadyRunning").Infof(ctx, "Skip '%s': already running", qJob.GetNamespacedName())
// Don't requeue the job.
return false, nil
}
Expand All @@ -167,7 +167,7 @@ func (j jobCreatorImpl) validateReferences(ctx context.Context, qJob qjv1a1.Quar
for configMapName := range configMaps {
if err := j.client.Get(ctx, crc.ObjectKey{Name: configMapName, Namespace: qJob.Namespace}, configMap); err != nil {
if apierrors.IsNotFound(err) {
ctxlog.Debugf(ctx, "Skip create job '%s' due to configMap '%s' not found", qJob.Name, configMapName)
ctxlog.Debugf(ctx, "Skip create job '%s' due to configMap '%s' not found", qJob.GetNamespacedName(), configMapName)
}
return err
}
Expand All @@ -178,7 +178,7 @@ func (j jobCreatorImpl) validateReferences(ctx context.Context, qJob qjv1a1.Quar
for secretName := range secrets {
if err := j.client.Get(ctx, crc.ObjectKey{Name: secretName, Namespace: qJob.Namespace}, secret); err != nil {
if apierrors.IsNotFound(err) {
ctxlog.Debugf(ctx, "Skip create job '%s' due to secret '%s' not found", qJob.Name, secretName)
ctxlog.Debugf(ctx, "Skip create job '%s' due to secret '%s' not found", qJob.GetNamespacedName(), secretName)
}
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/kube/controllers/quarksjob/job_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ func (r *ReconcileJob) Reconcile(request reconcile.Request) (reconcile.Result, e
qj := qjv1a1.QuarksJob{}
err = r.client.Get(ctx, types.NamespacedName{Name: parentName, Namespace: instance.GetNamespace()}, &qj)
if err != nil {
return reconcile.Result{}, errors.Wrapf(err, "getting parent quarksJob in Job Reconciler for job %s", instance.GetName())
return reconcile.Result{}, errors.Wrapf(err, "getting parent quarksJob in Job Reconciler for job '%s/%s'", request.Namespace, instance.GetName())
}

// Delete Job if it succeeded
if instance.Status.Succeeded == 1 {
ctxlog.WithEvent(&qj, "DeletingJob").Infof(ctx, "Deleting succeeded job '%s'", instance.Name)
ctxlog.WithEvent(&qj, "DeletingJob").Infof(ctx, "Deleting succeeded job '%s/%s'", request.Namespace, instance.Name)
err = r.client.Delete(ctx, instance)
if err != nil {
ctxlog.WithEvent(instance, "DeleteError").Errorf(ctx, "Cannot delete succeeded job: '%s'", err)
Expand All @@ -100,7 +100,7 @@ func (r *ReconcileJob) Reconcile(request reconcile.Request) (reconcile.Result, e
ctxlog.WithEvent(instance, "NotFoundError").Errorf(ctx, "Cannot find job's pod: '%s'", err)
return reconcile.Result{}, nil
}
ctxlog.WithEvent(&qj, "DeletingJobsPod").Infof(ctx, "Deleting succeeded job's pod '%s'", pod.Name)
ctxlog.WithEvent(&qj, "DeletingJobsPod").Infof(ctx, "Deleting succeeded job's pod '%s/%s'", pod.Namespace, pod.Name)
err = r.client.Delete(ctx, pod)
if err != nil {
ctxlog.WithEvent(instance, "DeleteError").Errorf(ctx, "Cannot delete succeeded job's pod: '%s'", err)
Expand All @@ -122,10 +122,10 @@ func (r *ReconcileJob) jobPod(ctx context.Context, name string, namespace string
client.MatchingLabels(map[string]string{"job-name": name}),
)
if err != nil {
return nil, errors.Wrapf(err, "Listing job's %s pods failed.", name)
return nil, errors.Wrapf(err, "Listing job's '%s/%s' pods failed.", namespace, name)
}
if len(list.Items) == 0 {
return nil, errors.Errorf("Job %s does not own any pods?", name)
return nil, errors.Errorf("Job '%s/%s' does not own any pods?", namespace, name)
}

// If there is only one job pod, then return index 0 pod.
Expand All @@ -143,6 +143,6 @@ func (r *ReconcileJob) jobPod(ctx context.Context, name string, namespace string
}
}

ctxlog.Infof(ctx, "Considering job pod %s for persisting output", latestPod.GetName())
ctxlog.Infof(ctx, "Considering job pod '%s/%s' for persisting output", namespace, latestPod.GetName())
return &latestPod, nil
}
4 changes: 2 additions & 2 deletions pkg/kube/controllers/quarksjob/job_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ var _ = Describe("ReconcileJob", func() {
_, err := reconciler.Reconcile(request)
Expect(err).ToNot(HaveOccurred())
Expect(logs.FilterMessageSnippet("Cannot find job's pod").Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Listing job's %s pods failed.", job.Name)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Listing job's '/%s' pods failed.", job.Name)).Len()).To(Equal(1))
})

It("handles an error when pod list is empty", func() {
Expand All @@ -271,7 +271,7 @@ var _ = Describe("ReconcileJob", func() {
_, err := reconciler.Reconcile(request)
Expect(err).ToNot(HaveOccurred())
Expect(logs.FilterMessageSnippet("Cannot find job's pod").Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Job %s does not own any pods?", job.Name)).Len()).To(Equal(1))
Expect(logs.FilterMessageSnippet(fmt.Sprintf("Job '/%s' does not own any pods?", job.Name)).Len()).To(Equal(1))
})
})

Expand Down

0 comments on commit b4e40a6

Please sign in to comment.