feat(csv): detect changes to a deployment and persist them #931
Conversation
openshift-ci-robot
added
approved
|
/retest |
ecordell
force-pushed
the
depupdate
branch
2 times, most recently
from
40bbd18 to
8430e05
Compare
|
/retest |
1 similar comment
|
/retest |
|
/refresh |
|
/retest |
|
/retest |
1 similar comment
|
/retest |
| // check equality | ||
| calculated := i.deploymentForSpec(spec.Name, spec.Spec) | ||
| if !i.equalDeployments(&calculated.Spec, &dep.Spec) { | ||
| return StrategyError{Reason: StrategyErrDeploymentUpdated, Message: fmt.Sprintf("deployment changed, rolling update with patch: %s\n%#v\n%#v", diff.ObjectDiff(dep.Spec.Template.Spec, calculated.Spec.Template.Spec), calculated.Spec.Template.Spec, dep.Spec.Template.Spec)} |
There was a problem hiding this comment.
Question: I assume this error will cause the csv to go to a Failed state for a retry - in the next sync loop it will be set to Pending and redeployment will take place from there, correct?
There was a problem hiding this comment.
IIRC, we don't take a step through Failed if we know that a re-apply can fix it. It should go straight back to Pending for re-install.
ecordell
force-pushed
the
depupdate
branch
2 times, most recently
from
291f11f to
b22c6bb
Compare
| @@ -148,7 +148,7 @@ func newNginxDeployment(name string) appsv1.DeploymentSpec { | |||
| Containers: []corev1.Container{ | |||
| { | |||
| Name: genName("nginx"), | |||
| Image: "bitnami/nginx:latest", | |||
| Image: "redis", | |||
There was a problem hiding this comment.
🙂
Note: It would be nice not to worry about issues with external images. Maybe, in the future, we could use a standard base image configured to run a "generic test operator" script.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ecordell, njhale The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Prior to this PR, changes made to a CSV's deployment specs would not affect the running deployments. Instead, you would need to delete and recreate the CSV, or create a new one that replaces the existing one, to test changes to a deployment.
This is obviously a UX improvement for developing CSVs, but this also has an advantage for simplifying our deployment. We are looking at switching to kustomize instead of helm, which means we need to limit the templating we do, and specifically the lack of partials in kustomize means that it's difficult to support our current deployment model. With this change we can just include the packageserver CSV (instead of CSV + Deployment + CatalogSource + ConfigMap) in our release artifacts.