feat(crd): add disableContainerDeployments flag - #683
Conversation
db8a03e to
841bbfb
Compare
Allow FrontendEnvironments to skip Deployments, Services, Jobs, and ServiceMonitors while still creating ConfigMaps and Ingress. RHCLOUD-41034 Co-authored-by: Cursor <cursoragent@cursor.com>
fixup! feat(crd): add disableContainerDeployments flag
4476d58 to
e65a9f6
Compare
karelhala
left a comment
karelhala
left a comment
There was a problem hiding this comment.
Couple of notes on Job re-enable and the Ingress backend when the flag is on.
| if skipUpdate { | ||
| return r.Cache.Update(ident, j) | ||
| } |
There was a problem hiding this comment.
skipUpdate is true for any Job that still exists and matches the current image, including one with deletionTimestamp set.
After disable toggles on, cache.Reconcile deletes these Jobs. A quick re-enable then Gets the terminating Job, Cache.Updates it unchanged (Apply skips), and the reconcile succeeds. The controller does not Owns Jobs, so when the Job is actually gone nothing enqueues the Frontend again — cachebust/pushcache never come back until some unrelated event.
The toggle test only gets past this by force-deleting Jobs and clearing finalizers first (frontend_controller_suite_test.go around the "Force-removing terminating Jobs" step).
Treat a deleting Job as not skippable so this reconcile errors and requeues:
| if skipUpdate { | |
| return r.Cache.Update(ident, j) | |
| } | |
| if skipUpdate { | |
| if !j.GetDeletionTimestamp().IsZero() { | |
| return fmt.Errorf("job %s is terminating, will retry", jobName) | |
| } | |
| return r.Cache.Update(ident, j) | |
| } |
Also worth Owns(&batchv1.Job{}) so a completed deletion retriggers even if this window is missed.
| // Default to the Frontend name (operator-managed Service). When container | ||
| // deployments are disabled or no image is set, prefer Spec.Service if | ||
| // provided; otherwise keep Frontend.Name so the Ingress backend is never empty. | ||
| serviceName := nn.Name |
There was a problem hiding this comment.
When the flag is on and Spec.Service is empty, the backend stays nn.Name (the Frontend name) even though this reconcile no longer creates that Service.
The e2e case does the same (chrome Ingress → chrome Service, no Service object). Is the contract “callers always set spec.service to an external Service”, “a same-named Service exists out of band”, or should we log/skip Ingress when there is nothing to point at? As written this Ingress will 502 unless something else creates that Service.
2 participants
Summary
disableContainerDeploymentsonFrontendEnvironmentto skip Deployments, Services, Jobs, and ServiceMonitors for all Frontends in the environmentSpec.Servicewhen set, otherwise falls back to the Frontend nameTest plan
DisableContainerDeploymentssuite (flag from start, toggle Deploy/Service/Ingress backend, Jobs cleanup+recreate, ServiceMonitor cleanup, Spec.Service fallback, Frontend.Spec.Disabled precedence)tests/e2e/disable-container-deployments(Ingress + ConfigMap present, no Deployment/Service)https://issues.redhat.com/browse/RHCLOUD-41034