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

✨ Move cluster-specifics from Manager into new pkg/cluster #1307

Merged
merged 1 commit into from
Jan 6, 2021

Conversation

alvaroaleman
Copy link
Member

This change is the first step towards implementing the corresponding
proposal. It essentially consists of moving a bunch if code from
pkg/manager into pkg/cluster, slightly adjusting some tests in
pkg/manager and copying applicable tests from there into pkg/cluster.

It is not yet the full implementation, because the Manager will only
correctly start it's own cache and not other caches before starting the
remaining Runnables. This behavior matches current behavior when
building something that uses multiple caches and will be fixed in a
follow-up.

This change is the first step towards implementing the corresponding
[proposal][1]. It essentially consists of moving a bunch if code from
pkg/manager into pkg/cluster, slightly adjusting some tests in
pkg/manager and copying applicable tests from there into pkg/cluster.

It is not yet the full implementation, because the `Manager` will only
correctly start it's own cache and not other caches before starting the
remaining `Runnables`. This behavior matches current behavior when
building something that uses multiple caches and will be fixed in a
follow-up.

[1]: https://github.com/kubernetes-sigs/controller-runtime/blob/master/designs/move-cluster-specific-code-out-of-manager.md
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 23, 2020
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 23, 2020
@alvaroaleman
Copy link
Member Author

Tests are failing because they still run with go 1.13 and we need at least 1.14 to get https://golang.org/doc/go1.14#language
kubernetes/test-infra#20316 will fix that

All tests succeed when running them locally with go 1.15:

$ g rev-parse HEAD && ./hack/check-everything.sh 
066bfea7df288d72eba0fcf98e1c21d72d8ec31b
fetching envtest tools@1.19.2 (into '/tmp/kubebuilder')
kubebuilder/bin/
kubebuilder/bin/etcd
kubebuilder/bin/kube-apiserver
kubebuilder/bin/kubectl
fetching envtest tools@1.19.2 (into './hack/../pkg/internal/testing/integration/assets')
kubebuilder/bin/
kubebuilder/bin/etcd
kubebuilder/bin/kube-apiserver
kubebuilder/bin/kubectl
setting up env vars
running generate
cd hack/tools && go build -tags=tools -o bin/controller-gen sigs.k8s.io/controller-tools/cmd/controller-gen
hack/tools/bin/controller-gen object paths="./pkg/config/v1alpha1/...;./examples/configfile/custom/v1alpha1/..."
running golangci-lint
/home/alvaro/git/golang/src/sigs.k8s.io/controller-runtime/hack/tools/bin/golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/alvaro/git/golang/src/sigs.k8s.io/controller-runtime /home/alvaro/git/golang/src/sigs.k8s.io /home/alvaro/git/golang/src /home/alvaro/git/golang /home/alvaro/git /home/alvaro /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 15 linters: [deadcode dupl errcheck goconst gocyclo goimports golint govet ineffassign lll misspell nakedret structcheck unparam varcheck] 
INFO [lintersdb] Active 15 linters: [deadcode dupl errcheck goconst gocyclo goimports golint govet ineffassign lll misspell nakedret structcheck unparam varcheck] 
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|imports|types_sizes|exports_file|files|name) took 1.176958092s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 36.749181ms 
INFO [runner/goanalysis_metalinter/goanalysis] analyzers took 0s with no stages 
INFO [runner/skip dirs] Skipped 1 issues from dir examples/configfile/builtin by pattern (^|/)examples($|/) 
INFO [runner/skip dirs] Skipped 3 issues from dir examples/crd/pkg by pattern (^|/)examples($|/) 
INFO [runner/skip dirs] Skipped 2 issues from dir examples/builtins by pattern (^|/)examples($|/) 
INFO [runner/skip dirs] Skipped 2 issues from dir examples/configfile/custom by pattern (^|/)examples($|/) 
INFO [runner] Issues before processing: 20, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_dirs: 12/20, identifier_marker: 11/11, exclude-rules: 0/11, filename_unadjuster: 20/20, skip_files: 20/20, exclude: 11/11, cgo: 20/20, path_prettifier: 20/20, autogenerated_exclude: 11/12 
INFO [runner] processing took 699.659µs with stages: path_prettifier: 247.715µs, identifier_marker: 173.292µs, skip_dirs: 153.779µs, autogenerated_exclude: 59.115µs, exclude-rules: 54.087µs, cgo: 4.416µs, filename_unadjuster: 2.744µs, max_same_issues: 1.134µs, nolint: 780ns, max_from_linter: 466ns, diff: 431ns, skip_files: 343ns, uniq_by_line: 323ns, source_code: 296ns, exclude: 288ns, max_per_file_from_linter: 262ns, path_shortener: 188ns 
INFO [runner] linters took 243.676365ms with stages: goanalysis_metalinter: 242.89809ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 16 samples, avg is 84.0MB, max is 138.3MB 
INFO Execution took 1.462513348s                  
verifying modules
go mod tidy
cd hack/tools; go mod tidy
running go test
ok  	sigs.k8s.io/controller-runtime	0.116s [no tests to run]
?   	sigs.k8s.io/controller-runtime/examples/builtins	[no test files]
?   	sigs.k8s.io/controller-runtime/examples/configfile/builtin	[no test files]
?   	sigs.k8s.io/controller-runtime/examples/configfile/custom	[no test files]
?   	sigs.k8s.io/controller-runtime/examples/configfile/custom/v1alpha1	[no test files]
?   	sigs.k8s.io/controller-runtime/examples/crd	[no test files]
?   	sigs.k8s.io/controller-runtime/examples/crd/pkg	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/builder	16.257s
ok  	sigs.k8s.io/controller-runtime/pkg/cache	76.595s
?   	sigs.k8s.io/controller-runtime/pkg/cache/informertest	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/cache/internal	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/client	26.301s
ok  	sigs.k8s.io/controller-runtime/pkg/client/apiutil	1.592s
ok  	sigs.k8s.io/controller-runtime/pkg/client/config	0.097s
ok  	sigs.k8s.io/controller-runtime/pkg/client/fake	0.394s
ok  	sigs.k8s.io/controller-runtime/pkg/cluster	16.843s
ok  	sigs.k8s.io/controller-runtime/pkg/config	0.095s
?   	sigs.k8s.io/controller-runtime/pkg/config/v1alpha1	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/controller	20.875s
?   	sigs.k8s.io/controller-runtime/pkg/controller/controllertest	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/controller/controllerutil	15.719s
?   	sigs.k8s.io/controller-runtime/pkg/conversion	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/envtest	54.736s
?   	sigs.k8s.io/controller-runtime/pkg/envtest/printer	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/event	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/handler	23.537s
ok  	sigs.k8s.io/controller-runtime/pkg/healthz	0.069s
ok  	sigs.k8s.io/controller-runtime/pkg/internal/controller	18.227s
?   	sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/internal/log	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/internal/objectutil	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/internal/recorder	17.635s
ok  	sigs.k8s.io/controller-runtime/pkg/internal/testing/integration	0.133s
ok  	sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr	0.047s
ok  	sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal	4.417s
ok  	sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal/integration_tests	129.052s
?   	sigs.k8s.io/controller-runtime/pkg/leaderelection	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/leaderelection/fake	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/log	0.052s
ok  	sigs.k8s.io/controller-runtime/pkg/log/zap	0.080s
ok  	sigs.k8s.io/controller-runtime/pkg/manager	15.128s
ok  	sigs.k8s.io/controller-runtime/pkg/manager/signals	1.051s
?   	sigs.k8s.io/controller-runtime/pkg/metrics	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/patterns/application	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/patterns/operator	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/predicate	0.215s
?   	sigs.k8s.io/controller-runtime/pkg/ratelimiter	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/reconcile	0.051s
ok  	sigs.k8s.io/controller-runtime/pkg/recorder	0.055s [no tests to run]
?   	sigs.k8s.io/controller-runtime/pkg/runtime	[no test files]
ok  	sigs.k8s.io/controller-runtime/pkg/runtime/inject	0.107s
ok  	sigs.k8s.io/controller-runtime/pkg/scheme	0.068s
ok  	sigs.k8s.io/controller-runtime/pkg/source	11.157s
ok  	sigs.k8s.io/controller-runtime/pkg/source/internal	0.164s
ok  	sigs.k8s.io/controller-runtime/pkg/webhook	14.800s
ok  	sigs.k8s.io/controller-runtime/pkg/webhook/admission	0.265s
ok  	sigs.k8s.io/controller-runtime/pkg/webhook/conversion	0.214s
?   	sigs.k8s.io/controller-runtime/pkg/webhook/internal/certwatcher	[no test files]
?   	sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics	[no test files]
confirming examples compile (via go install)
passed

@alvaroaleman
Copy link
Member Author

/test pull-controller-runtime-test-master

Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/milestone v0.8.x

@k8s-ci-robot k8s-ci-robot added this to the v0.8.x milestone Jan 6, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 6, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, vincepri

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:
  • OWNERS [alvaroaleman,vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants