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

Remove support for kubernetes-deploy.shopify.io annotations #526

Closed
dturn opened this issue Aug 14, 2019 · 7 comments · Fixed by #738
Closed

Remove support for kubernetes-deploy.shopify.io annotations #526

dturn opened this issue Aug 14, 2019 · 7 comments · Fixed by #738
Assignees

Comments

@dturn
Copy link
Contributor

dturn commented Aug 14, 2019

Problem

We're transitioning from kubernetes-deploy to krane. This means that we need to stop supporting kubernetes-deploy.shopify.io annotations. This is the final step and should be done no sooner than the 1.0 release and it may be desirable to do this after the 1.0 release

Possible solution

Remove all instances from the codebase.

Alternative solutions

Old annotations raise errors.

Acceptance criteria

  • kubernetes-deploy.shopify.io annotations no longer work.
  • krane.shopify.io annotations continue to work.

Additional context

#256 (comment)

@RyanBrushett
Copy link
Contributor

While we're doing this, whomever picks this issue up should be mindful to make the annotations grepable again.

E.g. Right now, finding the prunable annotation in code requires finding the prunable? method and realizing it calls the krane_annotation methods which is not ideal.

This was done to easily allow support for two sets of supported annotations but they should really be made constants again, kind of like this but tidied up.

@ghost ghost added 🚀 1.0 requirement https://github.com/Shopify/kubernetes-deploy/issues/229 and removed 🚀 1.0 requirement https://github.com/Shopify/kubernetes-deploy/issues/229 labels Oct 3, 2019
@dturn dturn mentioned this issue Nov 18, 2019
@ghost ghost added the krane [ProdX-GSD] label Dec 10, 2019
@dturn
Copy link
Contributor Author

dturn commented Feb 18, 2020

We should stop annotating ejson secrets when we action this issue. The annotations aren't being used by any code paths, though we should be sure to check with K8s#Krane to ensure external users aren't relying on it.

This came up because in pre.1 the annotations still use the k8s-deploy prefix and that's causing the k8s-deploy annotation depreciations warning to trigger. This sucks since users can't fix it without going to 1.0, which is a managed env like ship-it is out of the user's control.

@KnVerey
Copy link
Contributor

KnVerey commented Feb 18, 2020

Is the warning about the ejson annotation we force actually fixed by moving 1.0? The annotation still uses kubernetes-deploy on master: https://github.com/Shopify/krane/blob/master/lib/krane/ejson_secret_provisioner.rb#L15

breunigs pushed a commit to breunigs/krane that referenced this issue Mar 31, 2020
The gem currently sets an annotation on Secret that is deprecated by itself
and therefore generates a warning. To avoid the confusion, we disable this
warning until the problem is fixed upstream:
See Shopify#526
@airhorns
Copy link
Contributor

airhorns commented Apr 9, 2020

Is it time for this maybe?

@dturn
Copy link
Contributor Author

dturn commented Apr 9, 2020

@airhorns the project is ready for it. Are you willing to PR it?

@xaviablaza
Copy link

Could we move forward with this deprecation with #587?

@dturn
Copy link
Contributor Author

dturn commented Jun 3, 2020

@xaviablaza Thanks for reminding us about this issue. We're ready to move forward, but it would be better if we handled all removing all of the old kubernetes-deploy annotations at once. Is that something you'd be willing to PR? If not even rebasing the PR you linked would be a big help. If you're unable we've got the work scheduled but I can't commit us to a definitive date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants