Remove all the deployments in the older version #257
Conversation
We can know if there is an older version of operator CR, by checking Status.Version. If it is not empty and not equal to the current version, we can delete the old resources with the label "serving.knative.dev/release=<oldVersion>".
There was a problem hiding this comment.
@houshengbo: 0 warnings.
In response to this:
We can know if there is an older version of operator CR, by checking
Status.Version. If it is not empty and not equal to the current version,
we can delete the old resources with the label
"serving.knative.dev/release=".Fixes #256
Proposed Changes
Release Note
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: houshengbo 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 |
|
What is the expected behavior? Is this supposed to remove „outdated“ deployments i.e. those that are no longer used in the new version? |
|
@markusthoemmes Correct. I am also seeking to remove some other older resources, which are not used any more in the new version, like service, configmaps, gateway, etc, but I did not find the solution to remove all of them will a single call. |
| deploymentList, err := r.KubeClientSet.AppsV1().Deployments(instance.GetNamespace()).List(listOptions) | ||
| if err == nil { | ||
| for _, dep := range deploymentList.Items { | ||
| if e := r.KubeClientSet.AppsV1().Deployments(instance.GetNamespace()).Delete(dep.Name, |
There was a problem hiding this comment.
This makes me nervous. Deleting a deployment instead of updating it introduces the potential for losing requests, I think. Besides, I prefer seeing in this function exactly which resources are obsolete and need to be deleted.
There was a problem hiding this comment.
@jcrossley3 To find out all the resources to be removed, I can think of using the manifestival package to read the manifest of the older version, and compare to the manifest of the new version, to see if they still exist in the new version. Do you have any other suggestions?
|
We can use an alternative way to delete the obsolete resources with the existing Based on the nature of the low frequency that a resource is deleted, we can do easy diff to find out the missing resource and maintain |
Fixes #256
Proposed Changes
Release Note