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

CronJob "Create Job" may render too long Job names #18846

Closed
3 tasks done
reiniertimmer opened this issue Jun 28, 2024 · 4 comments · Fixed by #19137
Closed
3 tasks done

CronJob "Create Job" may render too long Job names #18846

reiniertimmer opened this issue Jun 28, 2024 · 4 comments · Fixed by #19137
Labels
bug Something isn't working component:ui User interfaces bugs and enhancements type:bug

Comments

@reiniertimmer
Copy link
Contributor

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

When starting a Job from a CronJob in the ArgoCD UI, the Job is created by appending the date string %Y%m%d%H%M to the CronJob .metadata.name.

(See https://github.com/argoproj/argo-cd/blob/master/resource_customizations/batch/CronJob/actions/create-job/action.lua#L39)

This adds 13 characters to the CronJob name to create the Job object. However, according to the Kubernetes CronJob spec, when Kubernetes creates a Job from a CronJob, it will only add 11 characters, so the CronJob can be up until 52 characters long.

When creating a Job through ArgoCD UI "Create Job" option will produce the following error:

Unable to execute resource action: Job.batch "***-202406281143" is invalid: [metadata.labels: Invalid value: "***-202406281143": must be no more than 63 characters, spec.template.labels: Invalid value: "***-202406281143": must be no more than 63 characters]

Which makes sense, when the CronJob is 52 characters long, ArgoCD will create a Job with a name of 65 characters long, which will produce the above error.

To Reproduce

  • Create an Application in ArgoCD that creates a CronJob, with a name of length 51-52
  • Manually create a Job from that CronJob using the UI

image

This will show the above error.

Expected behavior

A Job will be started from the CronJob immediately with a correct name.

Recommendations

Replacing %Y (4-digit year) in the date format to %y (2-digit year), will create a Job with CronJob name +11 characters (which is conveniently exactly within spec).

So, changing:

  • %Y%m%d%H%M to
  • %y%m%d%H%M

in the following file: https://github.com/argoproj/argo-cd/blob/master/resource_customizations/batch/CronJob/actions/create-job/action.lua#L39

Version

(server version only, commandline not available)

{
    "Version": "v2.11.3+3f344d5",
    "BuildDate": "2024-06-06T08:42:00Z",
    "GitCommit": "3f344d54a4e0bbbb4313e1c19cfe1e544b162598",
    "GitTreeState": "clean",
    "GoVersion": "go1.21.9",
    "Compiler": "gc",
    "Platform": "linux/amd64",
    "KustomizeVersion": "v5.2.1 2023-10-19T20:13:51Z",
    "HelmVersion": "v3.14.4+g81c902a",
    "KubectlVersion": "v0.26.11",
    "JsonnetVersion": "v0.20.0"
}
@reiniertimmer reiniertimmer added the bug Something isn't working label Jun 28, 2024
@crenshaw-dev
Copy link
Member

Seems like a safe, reasonable change. Wanna open a PR?

@crenshaw-dev
Copy link
Member

Since it's kind of an edge case, let's ship it 2.13 and add a release note, just in case people are depending on the current format.

People can always override the action if they need the fix sooner.

@christianh814
Copy link
Member

I wonder if having the date in the Job "matters"...meaning, what if we switched to a UUID format? Just a thought. But I agree to maybe not change it right away.

@reiniertimmer
Copy link
Contributor Author

reiniertimmer commented Jul 22, 2024

Seems like a safe, reasonable change. Wanna open a PR?

Hi @crenshaw-dev yes, I made a PR for this

I wonder if having the date in the Job "matters"...meaning, what if we switched to a UUID format? Just a thought. But I agree to maybe not change it right away.

@christianh814 I don't think the current naming is documented anywhere, at least I could not find it in the related discussion for this feature #4116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:ui User interfaces bugs and enhancements type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants