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

Truncate cronjob name at 52 characters #1208

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Conversation

mseiwald
Copy link
Contributor

K8s cronjobs must be no longer than 52 characters. This fixes the issue of backup cronjobs not being created for DBs with long names.

@FxKu
Copy link
Member

FxKu commented Nov 11, 2020

Can you add a simple unit test for your new function? And maybe call it trim and not truncate?

@mseiwald mseiwald force-pushed the master branch 2 times, most recently from 6cb7a04 to e3d378c Compare November 11, 2020 10:46
@mseiwald
Copy link
Contributor Author

Done

@Jan-M
Copy link
Member

Jan-M commented Nov 11, 2020

We see several objects where the cluster name length is a problem.

Internally we have discussed other options also to ensure uniqueness for child objects, e.g. the connection pooler deployment potentially leveraging the "uid" of the CRD object. But that also does not look nice in kubectl or other tools.

Just stripping it short may be a good start, but maybe not the best final solution.

@mseiwald
Copy link
Contributor Author

I see and understand that this is not the ideal solution. However, for us this is currently a bug which affects us and we cannot enable backups for one of our DBs without re-deploying it with a shorter name...

@mseiwald
Copy link
Contributor Author

Any news on this?

@FxKu FxKu modified the milestone: 1.6 Dec 16, 2020
@FxKu
Copy link
Member

FxKu commented Jan 7, 2021

@mseiwald we've just merged an option to specify a shorter prefix for the logical backup jobs with #1287. I think, this should also solve your issue.

@mseiwald
Copy link
Contributor Author

mseiwald commented Jan 12, 2021

@FxKu That's a good workaround but there could still be situations where the name is too long.

In any case I updated my PR and fixed the merge conflicts. If you want you can still consider it. Otherwise I would suggest that we close it.

@Jan-M
Copy link
Member

Jan-M commented Feb 15, 2021

👍

1 similar comment
@FxKu
Copy link
Member

FxKu commented Feb 15, 2021

👍

@FxKu FxKu merged commit 17da6bc into zalando:master Feb 15, 2021
@FxKu
Copy link
Member

FxKu commented Feb 15, 2021

Thanks @mseiwald for your contribution. You're right, it could happen that the name is still too long.

@FxKu FxKu added this to the 1.6.1 milestone Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants