Predeploy more resources #5
Conversation
I just pushed ca20352 to add some information to the README, I'm looking more to see if it would make sense to document this elsewhere! |
README.md
Outdated
| - `krane.shopify.io/predeployed`: Causes a Custom Resource to be deployed in the pre-deploy phase. | ||
| - _Compatibility_: Custom Resource Definition | ||
| - `krane.shopify.io/predeployed`: Causes a Custom Resource, Deployment, Service, or Job to be deployed in the pre-deploy phase. | ||
| - _Compatibility_: Custom Resource Definition, Deployment, Service, Job |
There was a problem hiding this comment.
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.
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.
| def kind | ||
| @definition["kind"] | ||
| end | ||
|
|
There was a problem hiding this comment.
Should we move these methods to the parent class, KubernetesResource, to reduce duplication? Additionally, it’s worth noting that the kind method already exists in the parent class
- Updated `predeploy_sequence` to specify resource groups for better clarity. - Modified `predeployed?` method logic to allow for default behavior based on resource type. - Introduced `default_to_predeployed?` method in multiple resource classes to standardize predeployment behavior. - Removed redundant `predeployed?` methods from `Deployment`, `Job`, and `Service` classes. - Added debug logging in integration tests for better traceability of resource groups during deployment.
What are you trying to accomplish with this PR?
This will allow more resources to be marked as predeployed, including Services, Jobs and Deployments. This will allow invoking Krane fewer times to deploy the same number of resources in the case where multiple Krane invocations would be necessary to deploy resources in a specific order, e.g. making sure a Redis instance is set up before a primary application without invoking Krane twice.
How is this accomplished?
This is accomplished by having the cluster resource discovery respect if the 'predeployed' annotation is set on Services, Jobs and Deployments. This includes checking if the 'predeployed' annotation is set on Services, Deployments and Jobs, and then having the resource deployer check if the predeployed annotation is set. If it is, the deploy task deploys Deployments and Services before the individual pods, and Jobs after the pods.
After all of that is said and done, Deployments, Services and Jobs will be deployed in 'Phase 3', before 'Phase 4' where the main deployment occurs.
What could go wrong?
This could unintentionally create race conditions if the annotations are set incorrectly on resources that shouldn't be predeployed, creating an out of order deployment.