Apply manifests from URL and await resources with readiness #48990
Conversation
This comment has been minimized.
This comment has been minimized.
|
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
|
Thanks! Seems reasonable to me, but I'll defer to @metacosm for a proper review |
|
@geoand I updated the documentation and added an integration test for the URL manifest application. My team are already building some things that absolutely need this improvement. I completely understand that people are on holiday and we need to be patient, but we would love to have this in the next Quarkus LTS release. Is there anything I can do to increase the chances that this makes it into the next LTS release? |
|
The next LTS will be 3.27, which will be a continuation of 3.26, so as long as it gets in by then, everything lines up |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } | ||
| } | ||
|
|
||
| private boolean isReadinessApplicable(HasMetadata item) { |
There was a problem hiding this comment.
I've created a PR: fabric8io/kubernetes-client#7206, we should use the implementation from the client's Readiness (accessible from the Config) instead, if / when the PR is merged and available.
There was a problem hiding this comment.
Ah perfect! That is indeed exactly why I copied the function over, because it wasn't available publicly
There was a problem hiding this comment.
This probably has to go through the whole cycle though of merge -> release -> upgrade in Quarkus, then we can change this?
Would it be possible to merge this PR with the copied over function for now and I'll make sure I'll make a followup PR once the function is available? So we can make the next LTS Quarkus release with these changes?
There was a problem hiding this comment.
The new version of fabric8 has been released and merged in, however I'm still not happy with the public API as it's non-static, I made a followup PR before we remove the copied method: fabric8io/kubernetes-client#7280
...nt/src/main/java/io/quarkus/kubernetes/client/deployment/DevServicesKubernetesProcessor.java
Outdated
Show resolved
Hide resolved
|
@metacosm I worked through all the comments |
Status for workflow
|
Status for workflow
|
|
Given this comment (#49415 (comment)), is there still anything that is blocking this PR? Anything I need to do or that still needs to happen? |
Not from my side. |
This PR is a follow-up of #48536 to include a few improvements I've noticed since experimenting with this new feature in Quarkus 3.25.0.CR1
The first is the ability to apply a manifest from a URL.. Often, one of the installation options for applications on Kubernetes is a single manifest that installs all related resources, e.g. https://kueue.sigs.k8s.io/docs/installation/#install-by-kubectl. You then don't want to add a 1MB YAML file to your resources folder and commit it to git, so it makes sense to apply directly from the URL.
Secondly is for startup to await resources that have readiness statuses (e.g. deployments). If you deploy, for example, a CRD controller (or any application for that fact), you often want it to be fully running before you start running tests or interact with it in dev mode. So it makes sense to delay startup until all manifests are ready. Besides that, there is another reason we want to do this that cannot be replaced by the user writing code themselves to await the deployments: Sometimes, manifests being deployed depend on it.
We, for example, have a setup where one of the manifests we deploy is a full CRD controller, and we then follow it up with a custom resource. This custom resource will, however, fail to apply until the deployment for the CRD controller is ready, and since both the CRD controller and custom resource are part of the Quarkus startup, you have no way to control this startup process as a user without Quarkus awaiting the CRD deployment.
The strategy for awaiting resources is as follows: We always deploy all resources within a single manifest without any await in between. We then await between manifest deployments for the resources of the manifest. This makes most sense as manifests may depend on each other, but the internal resources of a single manifest will never depend on a readiness await (since then
kubectl apply -f manifest.yamlwould naturally fail, which means the manifest itself is just broken).