-
Notifications
You must be signed in to change notification settings - Fork 17
✨ Rebase kcp modifications onto v0.16.2 #42
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
presubmits: | ||
- name: pull-controller-runtime-everything | ||
always_run: true | ||
decorate: true | ||
clone_uri: "ssh://git@github.com/kcp-dev/controller-runtime.git" | ||
labels: | ||
preset-goproxy: "true" | ||
spec: | ||
containers: | ||
- image: ghcr.io/kcp-dev/infra/build:1.20.9-1 | ||
command: | ||
- make | ||
- test |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
approvers: | ||
- davidfestal | ||
- ncdc | ||
- stevekuznetsov | ||
- sttts | ||
reviewers: | ||
- fabianvf | ||
- varshaprasad96 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,10 +73,11 @@ $(GO_APIDIFF): $(TOOLS_DIR)/go.mod # Build go-apidiff from tools folder. | |
$(CONTROLLER_GEN): $(TOOLS_DIR)/go.mod # Build controller-gen from tools folder. | ||
cd $(TOOLS_DIR) && go build -tags=tools -o bin/controller-gen sigs.k8s.io/controller-tools/cmd/controller-gen | ||
|
||
$(GOLANGCI_LINT): .github/workflows/golangci-lint.yml # Download golanci-lint using hack script into tools folder. | ||
hack/ensure-golangci-lint.sh \ | ||
-b $(TOOLS_BIN_DIR) \ | ||
$(shell cat .github/workflows/golangci-lint.yml | grep "version: v" | sed 's/.*version: //') | ||
$(GOLANGCI_LINT): .github/workflows/golangci-lint.yml # Download golangci-lint using hack script into tools folder. | ||
GOBIN=$(abspath $(TOOLS_BIN_DIR)) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(shell cat .github/workflows/golangci-lint.yml | grep "version: v" | sed 's/.*version: //') | ||
|
||
.PHONY: tools | ||
tools: $(GO_APIDIFF) $(CONTROLLER_GEN) $(GOLANGCI_LINT) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we diverting here from upstream? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I carried these changes over from the kcp-1.26 branch, where the commits just said
If you want I can remove them and see if something breaks, as I am also not sure why exactly we added these. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No strong opinion, was just curious. |
||
|
||
## -------------------------------------- | ||
## Linting | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |
"net/http" | ||
"time" | ||
|
||
"github.com/kcp-dev/apimachinery/v2/third_party/informers" | ||
corev1 "k8s.io/api/core/v1" | ||
"k8s.io/apimachinery/pkg/api/meta" | ||
"k8s.io/apimachinery/pkg/fields" | ||
|
@@ -159,6 +160,14 @@ type Options struct { | |
// instead of `reconcile.Result{}`. | ||
SyncPeriod *time.Duration | ||
|
||
// NewInformerFunc is a function that is used to create SharedIndexInformers. | ||
// Defaults to cache.NewSharedIndexInformer from client-go | ||
NewInformerFunc client.NewInformerFunc | ||
|
||
// Indexers is the indexers that the informers will be configured to use. | ||
// Will always have the standard NamespaceIndex. | ||
Indexers toolscache.Indexers | ||
|
||
// ReaderFailOnMissingInformer configures the cache to return a ErrResourceNotCached error when a user | ||
// requests, using Get() and List(), a resource the cache does not already have an informer for. | ||
// | ||
|
@@ -201,9 +210,6 @@ type Options struct { | |
// ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object. | ||
// object, this will fall through to Default* settings. | ||
ByObject map[client.Object]ByObject | ||
|
||
// newInformer allows overriding of NewSharedIndexInformer for testing. | ||
newInformer *func(toolscache.ListerWatcher, runtime.Object, time.Duration, toolscache.Indexers) toolscache.SharedIndexInformer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure super why this is originally a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this different func type than |
||
} | ||
|
||
// ByObject offers more fine-grained control over the cache's ListWatch by object. | ||
|
@@ -354,7 +360,7 @@ func newCache(restConfig *rest.Config, opts Options) newCacheFunc { | |
}, | ||
Transform: config.Transform, | ||
UnsafeDisableDeepCopy: pointer.BoolDeref(config.UnsafeDisableDeepCopy, false), | ||
NewInformer: opts.newInformer, | ||
NewInformer: opts.NewInformerFunc, | ||
}), | ||
readerFailOnMissingInformer: opts.ReaderFailOnMissingInformer, | ||
} | ||
|
@@ -438,6 +444,10 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { | |
if opts.SyncPeriod == nil { | ||
opts.SyncPeriod = &defaultSyncPeriod | ||
} | ||
|
||
if opts.NewInformerFunc == nil { | ||
opts.NewInformerFunc = informers.NewSharedIndexInformer | ||
} | ||
return opts, nil | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should change this, follow-up or here.