Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix e2e test module component versions #1032

Merged
merged 4 commits into from
Aug 25, 2023

Conversation

mihaialexandrescu
Copy link
Contributor

Description

Currently, the tests/e2e module introduced dependencies on k8s components from v0.27.2 instead of the v0.26.4 we have in the other modules of this repo.

Beyond not being in line with the supported k8s versions, this introduced a problem visible in testing (make test) and importing certain packages (e.g. some of our own packages even from this repo like pkg/webhooks):

# sigs.k8s.io/controller-runtime/pkg/cache
../../../../pkg/mod/sigs.k8s.io/[email protected]/pkg/cache/multi_namespace_cache.go:308:9: cannot use handles (variable of type map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration) as "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration value in return statement: map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)
../../../../pkg/mod/sigs.k8s.io/[email protected]/pkg/cache/multi_namespace_cache.go:321:9: cannot use handles (variable of type map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration) as "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration value in return statement: map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)
../../../../pkg/mod/sigs.k8s.io/[email protected]/pkg/cache/multi_namespace_cache.go:326:17: impossible type assertion: h.(map[string]toolscache.ResourceEventHandlerRegistration)
	map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)
make: *** [vet] Error 1

That turns out to be a common issue when upgrading to modules that depend on client-go >v0.27.0 and is described succinctly here and here.
The gist of it is (remember we are currently in v0.26.4 in our other modules):

The type ResourceEventHandlerRegistration is added to client-go from v0.26.0. But it is different with 0.27.0.

0.26.0:
type ResourceEventHandlerRegistration interface{}

0.26.4
type ResourceEventHandlerRegistration interface{}

0.27.0
type ResourceEventHandlerRegistration interface {
// HasSynced reports if both the parent has synced and all pre-sync
// events have been delivered.
HasSynced() bool
}

For this PR, I removed all the require stanzas that had only indirect dependencies in them from tests/e2e/go.mod and then ran go mod tidy on the tests/e2e module. Then I did a go mod tidy on the root module, too.

Additionally, I am upgrading the koperator and koperator/api dependencies to the latest non-dev releases (at the time of writing).

Additionally, I am upgrading the patch version of Terratest because ... it's available and it didn't cause any other dependency changes.

Type of Change

  • Other (please describe): dependency modification (mostly downgrade)

Checklist

@mihaialexandrescu mihaialexandrescu marked this pull request as ready for review August 10, 2023 09:40
@mihaialexandrescu mihaialexandrescu requested a review from a team as a code owner August 10, 2023 09:40
pregnor
pregnor previously approved these changes Aug 14, 2023
@mihaialexandrescu
Copy link
Contributor Author

mihaialexandrescu commented Aug 16, 2023

As part of this PR, would we also want to downgrade the go version in the tests/e2e/go.mod file from 1.20 to 1.19 (like we have in all other modules in this repo) ?

I don't think it's a must do functionally since eventually every other module would catch up eventually and the tests/e2e module only imports others from this repo (but it isn't imported by others).

|> % grep -irn '^go 1.[0-9][0-9]$' 
./go.mod:3:go 1.19
./tests/e2e/go.mod:3:go 1.20   <<
./properties/go.mod:3:go 1.19
./api/go.mod:3:go 1.19

Later edit: we agreed to downgrade to keep consistency -> done in 46c15b9

@mihaialexandrescu mihaialexandrescu merged commit da8a72b into master Aug 25, 2023
4 checks passed
@mihaialexandrescu mihaialexandrescu deleted the fix/e2etest_component_versions branch August 25, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants