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
Limit 52-character cronjob name validation to create #52967
Limit 52-character cronjob name validation to create #52967
Conversation
kernel panic flake |
func ValidateCronJobUpdate(job, oldJob *batch.CronJob) field.ErrorList { | ||
allErrs := apivalidation.ValidateObjectMetaUpdate(&job.ObjectMeta, &oldJob.ObjectMeta, field.NewPath("metadata")) | ||
allErrs = append(allErrs, ValidateCronJobSpec(&job.Spec, field.NewPath("spec"))...) | ||
// skip the 52-character name validation limit on update validation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to sunset this skip in a future release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could, although the migrate strategy is pretty deployment-specific (ranging from "not at all" to "every release" to "tools available but not auto-run"), so it's unclear when we'd know they'd all been cleared out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully the resolution to #52185 is "every release", but I agree with you re the status quo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hope is a prereq for all my upgrades
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liggitt how long do we want to wait before "fixing" the update part entirely? Is 1.10 a good time frame or 1.11?
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: liggitt, thockin No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
TestDevicePluginReRegistration flake |
Automatic merge from submit-queue (batch tested with PRs 53263, 52967, 53262, 52654, 53187). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Follow up to #52733
Related to #50850
Needed to allow old cronjobs to be updated/migrated/deleted (with foregroundPropagation)