Improve the upgrade tests for better verification of the resources #166
Conversation
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
houshengbo
force-pushed
the
check-prow-sources-controller
branch
3 times, most recently
from
77e555e to
8fe6d49
Compare
houshengbo
force-pushed
the
check-prow-sources-controller
branch
2 times, most recently
from
8fddb4b to
aff8d1a
Compare
houshengbo
force-pushed
the
check-prow-sources-controller
branch
3 times, most recently
from
eef255c to
d7bd787
Compare
houshengbo
changed the title
houshengbo
force-pushed
the
check-prow-sources-controller
branch
2 times, most recently
from
d8875db to
6263eff
Compare
| resource.SetName("containersources.sources.eventing.knative.dev") | ||
| if err := manifest.Client.Delete(resource); err != nil { | ||
| return err | ||
| // Remove the obsolete CRDs at 0.13 |
There was a problem hiding this comment.
Is it safe to delete these CRDs? I know there were some historic concerns about deleting them wholesale as part of uninstall.
| resource.SetName("cronjobsources.sources.eventing.knative.dev") | ||
| if err := manifest.Client.Delete(resource); err != nil { | ||
| return err | ||
| for _, crd := range crds { |
There was a problem hiding this comment.
Have we considered adding a 'delete manifest' where we house cleanups until we get something like the one-off Job's proposed in the Operator re-think? It would be nice to minimize the Operator code changes, imo.
test/resources/verify.go
Outdated
| expectedDeployments []string) { | ||
| dpList, err := clients.KubeClient.Kube.AppsV1().Deployments(names.Namespace).List(metav1.ListOptions{}) | ||
| assertEqual(t, err, nil) | ||
| assertEqual(t, len(dpList.Items), len(expectedDeployments)) |
There was a problem hiding this comment.
Can we use the names of the resources in the equality check? It'd be nice to know what resource leaked from the test failure.
| } | ||
| } | ||
| return false | ||
| // |
There was a problem hiding this comment.
delete or uncomment, please
houshengbo
force-pushed
the
check-prow-sources-controller
branch
3 times, most recently
from
8b8bc39 to
3f740da
Compare
houshengbo
force-pushed
the
check-prow-sources-controller
branch
from
3f740da to
89a8e74
Compare
|
/lgtm |
4 participants
Issue to be fixed
Fixes #
Proposed Changes
Release Note