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
Needs 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 |
Issue to be fixed
Fixes #
Proposed Changes
Release Note