forked from Shopify/krane
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Predeploy more resources #5
Draft
c-gerke
wants to merge
24
commits into
upstream-clone
Choose a base branch
from
predeploy-more-resources
base: upstream-clone
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+102
−41
Draft
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
737574c
Allow to recognize the predeployed annotation
oct8l 4cc4e62
Allow the mock resource to be designated as predeployed
oct8l a075097
Fix logic in deciding if something should be predeployed
oct8l 8746eb1
Add functionality in the discovery to respect the predeployed annotation
oct8l f4e0385
Add Deployments, Services and Jobs to be deployed after CRs
oct8l 9c539c9
Fix a couple tests to be up to speed with the new functionality
oct8l d70bd2d
Change test to work now that Services can be predeployed
oct8l ca20352
Add information to the README about resources that can be marked as '…
c-gerke 5b45f3b
Also allow Ingresses to respect the 'predeployed' annotation
c-gerke 52a608d
Remove duplicate 'kind' definition
c-gerke c598e1e
Refactor predeploy logic and enhance resource handling
c-gerke 873c050
Explicitly set the priority resources to be predeployed
c-gerke 876715a
Clean up (now) unused definitions
c-gerke 2f88b93
Add more resources to the 'after_crs' section of pre-deployment
c-gerke 3af0df3
Documentation updates
c-gerke f873ef1
Remove a missed definition
c-gerke 042fc44
Revert change to force Role and RoleBinding to be predeployed at the …
c-gerke bd5bc25
Allow CR to be set to predeployed independent of CRDs
c-gerke 8bbbe90
Fix line references that have changed
c-gerke 9839b6d
Quick and dirty fix to get tests and deployments working properly
c-gerke bbc989f
Remove accidental newline
c-gerke 6c4c6a9
Refactor predeployed logic for Kubernetes resources
c-gerke 5016db4
Move pod predeploy logic with the rest of the "required to predeploy"…
c-gerke bfd40a7
Remove the `default_to_predeployed` function from previous iterations…
c-gerke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This might be controversial upstream; the label originally got applied to the resource definition (class) not the concrete instance, so this is now conceptually mixed.
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.
I had been thinking about that as well, that this implementation/change was the result of taking a feature that existed with one purpose and kind of stealing or borrowing it to apply to different objects. I don't doubt it would be controversial upstream, and I'm ready to have those conversations on if this should even be merged upstream too!
I think it's a worthwhile feature/functional change though (I do realize I'm a little close to it to see it completely objectively), and there's at least one other person at some point who was looking for this type of functionality, so I think it's worth at least proposing and discussing if this is something that would be desired in its current state.