Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Our k8s provider mandates the use of backoffLimit <=3. #1424

Closed
arjuncreativo opened this issue Apr 5, 2021 · 6 comments · Fixed by #1468
Closed

Our k8s provider mandates the use of backoffLimit <=3. #1424

arjuncreativo opened this issue Apr 5, 2021 · 6 comments · Fixed by #1468
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@arjuncreativo
Copy link

How can I set backoffLimit for storage type elasticsearch under esIndexCleaner and dependencies

time="2021-04-05T08:05:30Z" level=error msg="failed to apply the changes" error="admission webhook \"xxxxxx.webhook\" denied the request: Job spec in CronJob must set 'backoffLimit' <= 3" execution="2021-04-05 08:05:30.031265563 +0000 UTC" instance=jaeger-tracing-prd namespace=xxxxxxxxx
@jpkrohling jpkrohling transferred this issue from jaegertracing/jaeger Apr 7, 2021
@github-actions github-actions bot added the needs-triage New issues, in need of classification label Apr 7, 2021
@jpkrohling jpkrohling added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed and removed needs-triage New issues, in need of classification labels Apr 7, 2021
@jpkrohling
Copy link
Contributor

Seems like a good first issue. Would you like to send a PR for this? You'd need to change this struct to accommodate the new field:

type JaegerEsIndexCleanerSpec struct {
// +optional
Enabled *bool `json:"enabled,omitempty"`
// +optional
NumberOfDays *int `json:"numberOfDays,omitempty"`
// +optional
Schedule string `json:"schedule,omitempty"`
// +optional
SuccessfulJobsHistoryLimit *int32 `json:"successfulJobsHistoryLimit,omitempty"`
// +optional
Image string `json:"image,omitempty"`
// +optional
TTLSecondsAfterFinished *int32 `json:"ttlSecondsAfterFinished,omitempty"`
// +optional
JaegerCommonSpec `json:",inline,omitempty"`
}

Make sure to add the same field to other places that also create cronjobs, such as the JaegerDependenciesSpec and JaegerCassandraCreateSchemaSpec.

You'll then be able to use it here:

return &batchv1beta1.CronJob{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: jaeger.Namespace,
Labels: commonSpec.Labels,
Annotations: commonSpec.Annotations,
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: jaeger.APIVersion,
Kind: jaeger.Kind,
Name: jaeger.Name,
UID: jaeger.UID,
Controller: &trueVar,
},
},
},
Spec: batchv1beta1.CronJobSpec{
Schedule: jaeger.Spec.Storage.EsIndexCleaner.Schedule,
SuccessfulJobsHistoryLimit: jaeger.Spec.Storage.EsIndexCleaner.SuccessfulJobsHistoryLimit,
JobTemplate: batchv1beta1.JobTemplateSpec{
Spec: batchv1.JobSpec{
Parallelism: &one,
TTLSecondsAfterFinished: jaeger.Spec.Storage.EsIndexCleaner.TTLSecondsAfterFinished,
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: util.Truncate(name, 63),
Image: util.ImageName(jaeger.Spec.Storage.EsIndexCleaner.Image, "jaeger-es-index-cleaner-image"),
Args: []string{strconv.Itoa(*jaeger.Spec.Storage.EsIndexCleaner.NumberOfDays), esUrls},
Env: util.RemoveEmptyVars(envs),
EnvFrom: envFromSource,
Resources: commonSpec.Resources,
VolumeMounts: commonSpec.VolumeMounts,
},
},
RestartPolicy: corev1.RestartPolicyNever,
Affinity: commonSpec.Affinity,
Tolerations: commonSpec.Tolerations,
SecurityContext: commonSpec.SecurityContext,
ServiceAccountName: account.JaegerServiceAccountFor(jaeger, account.EsIndexCleanerComponent),
Volumes: commonSpec.Volumes,
},
ObjectMeta: metav1.ObjectMeta{
Labels: commonSpec.Labels,
Annotations: commonSpec.Annotations,
},
},
},
},
},
}

Make sure to add a test and docs for this.

@chasekiefer
Copy link
Contributor

I'm giving this a try and am looking to finish up by modifying the docs. Should I open a separate PR into the documentation repository?

@jpkrohling
Copy link
Contributor

Yes, please create a new PR for the docs repo, linking to this PR here. The changes there should be applied only to the next-release directory. Thanks!

@chasekiefer
Copy link
Contributor

@jpkrohling So I've been working through writing the docs but it appears https://www.jaegertracing.io/docs/1.22/operator/ doesn’t dive very deep into the CRD options. It seems like only pretty high-level features are mentioned. Does this deserve a mention in the docs?

@jpkrohling
Copy link
Contributor

Right, sorry, it should probably be kept only as a godoc in the relevant Go type. We'll eventually figure out a way to properly document the whole CRD based on that.

@chasekiefer
Copy link
Contributor

I added the godoc but I had to open a new PR. It should be good to go now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
3 participants