Skip to content

Commit ec6651d

Browse files
committed
Implement ServiceNamespace creation and requeue logic in claimedresources controller
1 parent 902fcda commit ec6651d

12 files changed

Lines changed: 383 additions & 60 deletions

File tree

Makefile

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ CODE_GENERATOR_BIN := code-generator
7777
CODE_GENERATOR := $(TOOLS_GOBIN_DIR)/$(CODE_GENERATOR_BIN)-$(CODE_GENERATOR_VER)
7878
export CODE_GENERATOR # so hack scripts can use it
7979

80-
KCP_VER := v0.28.1
80+
KCP_VER := v0.28.0
8181
KCP_BIN := kcp
8282
KCP := $(TOOLS_GOBIN_DIR)/$(KCP_BIN)-$(KCP_VER)
8383
KCP_CMD ?= $(KCP)
@@ -245,8 +245,12 @@ GO_TEST = $(GOTESTSUM) $(GOTESTSUM_ARGS) --
245245
endif
246246

247247
COUNT ?= 1
248-
NPROC ?= $$(( $(shell nproc) / 2 ))
249-
E2E_PARALLELISM ?= $$(( $(NPROC) > 1 ? $(NPROC) : 1))
248+
# Only set parallelism if user specified E2E_PARALLELISM
249+
ifdef E2E_PARALLELISM
250+
E2E_PARALLELISM_FLAG := -p $(E2E_PARALLELISM) -parallel $(E2E_PARALLELISM)
251+
else
252+
E2E_PARALLELISM_FLAG :=
253+
endif
250254

251255
$(DEX):
252256
mkdir -p $(TOOLS_DIR)
@@ -281,15 +285,15 @@ test-e2e: $(KCP) $(DEX) build ## Run e2e tests
281285
$(MAKE) run-kcp &>.kcp/kcp.log & KCP_PID=$$!; \
282286
trap 'kill -TERM $$DEX_PID $$KCP_PID; rm -rf .kcp' TERM INT EXIT && \
283287
echo "Waiting for kcp to be ready (check .kcp/kcp.log)." && while ! KUBECONFIG=.kcp/admin.kubeconfig kubectl get --raw /readyz &>/dev/null; do sleep 1; echo -n "."; done && echo && \
284-
KUBECONFIG=$$PWD/.kcp/admin.kubeconfig GOOS=$(OS) GOARCH=$(ARCH) $(GO_TEST) -race -count $(COUNT) -p $(E2E_PARALLELISM) -parallel $(E2E_PARALLELISM) $(WHAT) $(TEST_ARGS)
288+
KUBECONFIG=$$PWD/.kcp/admin.kubeconfig GOOS=$(OS) GOARCH=$(ARCH) $(GO_TEST) -race -count $(COUNT) $(E2E_PARALLELISM_FLAG) $(WHAT) $(TEST_ARGS)
285289

286290
CONTRIBS_E2E := $(patsubst %,test-e2e-contrib-%,$(CONTRIBS))
287291

288292
.PHONY: test-e2e-contribs $(CONTRIBS_E2E)
289293
test-e2e-contribs: $(CONTRIBS_E2E) ## Run e2e tests for external integrations
290294
test-e2e-contrib-kcp: build $(KCP)
291295
$(CONTRIBS_E2E):
292-
cd contrib/$(patsubst test-e2e-contrib-%,%,$@) && $(GO_TEST) -race -count $(COUNT) -p $(E2E_PARALLELISM) -parallel $(E2E_PARALLELISM) ./test/e2e/...
296+
cd contrib/$(patsubst test-e2e-contrib-%,%,$@) && $(GO_TEST) -race -count $(COUNT) $(E2E_PARALLELISM_FLAG) ./test/e2e/...
293297

294298
.PHONY: test
295299
ifdef USE_GOTESTSUM

backend/controllers/serviceexportrequest/serviceexportrequest_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ func NewAPIServiceExportRequestReconciler(
8383
informerScope: scope,
8484
clusterScopedIsolation: isolation,
8585
schemaSource: schemaSource,
86-
getBoundSchema: func(ctx context.Context, cache cache.Cache, namespace, name string) (*kubebindv1alpha2.BoundSchema, error) {
86+
getBoundSchema: func(ctx context.Context, cl client.Client, namespace, name string) (*kubebindv1alpha2.BoundSchema, error) {
8787
var schema kubebindv1alpha2.BoundSchema
8888
key := types.NamespacedName{Namespace: namespace, Name: name}
89-
if err := cache.Get(ctx, key, &schema); err != nil {
89+
if err := cl.Get(ctx, key, &schema); err != nil {
9090
return nil, err
9191
}
9292
return &schema, nil

backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ type reconciler struct {
4444
clusterScopedIsolation kubebindv1alpha2.Isolation
4545
schemaSource string
4646

47-
getBoundSchema func(ctx context.Context, cache cache.Cache, namespace, name string) (*kubebindv1alpha2.BoundSchema, error)
47+
getBoundSchema func(ctx context.Context, cl client.Client, namespace, name string) (*kubebindv1alpha2.BoundSchema, error)
4848
createBoundSchema func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error
4949

5050
getServiceExport func(ctx context.Context, cache cache.Cache, ns, name string) (*kubebindv1alpha2.APIServiceExport, error)
@@ -57,17 +57,17 @@ func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cach
5757
// Worst case scenario if validation fails, we will reuse schemas for same consumer once issues are fixed.
5858
if err := r.ensureBoundSchemas(ctx, cl, cache, req); err != nil {
5959
conditions.SetSummary(req)
60-
return err
60+
return fmt.Errorf("failed to ensure bound schemas: %w", err)
6161
}
6262

6363
if err := r.validate(ctx, cl, req); err != nil {
6464
conditions.SetSummary(req)
65-
return err
65+
return fmt.Errorf("failed to validate APIServiceExportRequest: %w", err)
6666
}
6767

6868
if err := r.ensureExports(ctx, cl, cache, req); err != nil {
6969
conditions.SetSummary(req)
70-
return err
70+
return fmt.Errorf("failed to ensure exports: %w", err)
7171
}
7272

7373
// TODO(mjudeikis): we could potentially add finallizer to APIServiceExport above or "adopt" boundschemas
@@ -148,8 +148,8 @@ func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, c
148148
boundSchema.Spec.InformerScope = r.informerScope
149149
boundSchema.ResourceVersion = ""
150150

151-
obj, err := r.getBoundSchema(ctx, cache, boundSchema.Namespace, boundSchema.Name)
152-
if err != nil && !apierrors.IsNotFound(err) {
151+
obj, err := r.getBoundSchema(ctx, cl, boundSchema.Namespace, boundSchema.Name)
152+
if err != nil && !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
153153
return err
154154
}
155155

@@ -176,7 +176,7 @@ func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, cache
176176
if req.Status.Phase == kubebindv1alpha2.APIServiceExportRequestPhasePending {
177177
for _, res := range req.Spec.Resources {
178178
name := res.ResourceGroupName()
179-
boundSchema, err := r.getBoundSchema(ctx, cache, req.Namespace, name)
179+
boundSchema, err := r.getBoundSchema(ctx, cl, req.Namespace, name)
180180
if err != nil {
181181
if apierrors.IsNotFound(err) {
182182
conditions.MarkFalse(

backend/http/handler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ func (h *handler) getBackendDynamicResource(ctx context.Context, cluster string)
557557
}
558558
items, err := h.kubeManager.ListDynamicResources(ctx, cluster, gvk, labelSelector.AsSelector())
559559
if err != nil {
560-
return nil, fmt.Errorf("failed to list crds: %w", err)
560+
return nil, fmt.Errorf("failed to list resources: %w", err)
561561
}
562562
var boundSchemas kubebindv1alpha2.ExportedSchemas = make(map[string]*kubebindv1alpha2.BoundSchema, len(items.Items))
563563
for _, item := range items.Items {

contrib/kcp/README.md

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ k ws use :root:kube-bind
5151
--cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
5252
--cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
5353
--schema-source apiresourceschemas \
54-
--consumer-scope=namespaced
54+
--consumer-scope=cluster
5555
```
5656

5757

@@ -79,10 +79,9 @@ kubectl kcp bind apiexport root:kube-bind:kube-bind.io --accept-permission-claim
7979

8080
7. Create CRD in provider:
8181
```bash
82-
kubectl create -f kcp/deploy/examples/apiexport.yaml
83-
kubectl create -f kcp/deploy/examples/apiresourceschema-cowboys.yaml
84-
kubectl create -f kcp/deploy/examples/apiresourceschema-sheriffs.yaml
85-
# recursive bind
82+
kubectl create -f contrib/kcp/deploy/examples/apiexport.yaml
83+
kubectl create -f contrib/kcp/deploy/examples/apiresourceschema-cowboys.yaml
84+
kubectl create -f contrib/kcp/deploy/examples/apiresourceschema-sheriffs.yaml
8685
kubectl kcp bind apiexport root:provider:cowboys-stable
8786
```
8887

@@ -91,7 +90,7 @@ kubectl kcp bind apiexport root:provider:cowboys-stable
9190
```bash
9291
kubectl get logicalcluster
9392
# NAME PHASE URL AGE
94-
# cluster Ready https://192.168.2.166:6443/clusters/43d7su0lk1bxyaia
93+
# cluster Ready https://192.168.2.166:6443/clusters/crh63mkto0egdjca
9594
```
9695

9796
9. Now we gonna initiate consumer:
@@ -105,15 +104,15 @@ kubectl ws create consumer --enter
105104
10. Bind the thing:
106105

107106
```bash
108-
./bin/kubectl-bind http://127.0.0.1:8080/clusters/43d7su0lk1bxyaia/exports --dry-run -o yaml > apiserviceexport.yaml
107+
./bin/kubectl-bind http://127.0.0.1:8080/clusters/crh63mkto0egdjca/exports --dry-run -o yaml > apiserviceexport.yaml
109108

110109
# Extract secret for binding process. Note that secret name is not the same as output from command above. Check secret
111110
# name by running `kubectl get secret -n kube-bind`
112-
kubectl get secret kubeconfig-c88q6 -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
111+
kubectl get secret kubeconfig-npp6r -n kube-bind -o jsonpath='{.data.kubeconfig}' | base64 -d > remote.kubeconfig
113112

114-
./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f kcp/deploy/examples/apiserviceexport-namespaced.yaml --skip-konnector --remote-namespace kube-bind-tnnfp
113+
./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f contrib/kcp/deploy/examples/apiserviceexport-namespaced.yaml --skip-konnector --remote-namespace kube-bind-w6gs4
115114

116-
./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f kcp/deploy/examples/apiserviceexport-cluster.yaml --skip-konnector --remote-namespace kube-bind-tnnfp
115+
./bin/kubectl-bind apiservice --remote-kubeconfig remote.kubeconfig -f contrib/kcp/deploy/examples/apiserviceexport-cluster.yaml --skip-konnector --remote-namespace kube-bind-t6wg7
117116

118117

119118
export KUBECONFIG=.kcp/consumer.kubeconfig
@@ -122,8 +121,8 @@ go run ./cmd/konnector/ --lease-namespace default
122121

123122
Create objects:
124123
```
125-
kubectl apply -f kcp/deploy/examples/cowboy.yaml
126-
kubectl apply -f kcp/deploy/examples/sheriff.yaml
124+
kubectl apply -f contrib/kcp/deploy/examples/cowboy.yaml
125+
kubectl apply -f contrib/kcp/deploy/examples/sheriff.yaml
127126
```
128127

129128

contrib/kcp/deploy/bootstrap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func bindAPIExport(ctx context.Context, kcpClient kcpclient.Interface, exportNam
241241
Group: "apis.kcp.io",
242242
Resource: "apiresourceschemas",
243243
},
244-
Verbs: []string{"get", "list", "watch"},
244+
Verbs: []string{"*"},
245245
},
246246
Selector: apisv1alpha2.PermissionClaimSelector{
247247
MatchAll: true,

contrib/kcp/deploy/resources/apiexport-kube-bind.io.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ spec:
4040
- group: apis.kcp.io
4141
resource: apiresourceschemas
4242
verbs:
43-
- '*'
43+
- "*"
4444
resources:
4545
- group: kube-bind.io
4646
name: apiconversions

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ replace (
2929

3030
require (
3131
github.com/coreos/go-oidc/v3 v3.15.0
32+
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
3233
github.com/dexidp/dex/api/v2 v2.3.0
3334
github.com/evanphx/json-patch/v5 v5.9.11
3435
github.com/google/go-cmp v0.7.0
@@ -79,7 +80,6 @@ require (
7980
github.com/cespare/xxhash/v2 v2.3.0 // indirect
8081
github.com/coreos/go-semver v0.3.1 // indirect
8182
github.com/coreos/go-systemd/v22 v22.5.0 // indirect
82-
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
8383
github.com/emicklei/go-restful/v3 v3.12.1 // indirect
8484
github.com/evanphx/json-patch v5.9.11+incompatible // indirect
8585
github.com/fatih/color v1.18.0 // indirect

pkg/konnector/controllers/cluster/claimedresources/claimedresources_controller.go

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func NewController(
5858
consumerDynamicInformer informers.GenericInformer,
5959
providerDynamicInformer multinsinformer.GetterInformer,
6060
serviceNamespaceInformer dynamic.Informer[bindlisters.APIServiceNamespaceLister],
61+
namespaceCreationNotifyChan <-chan string,
6162
) (*controller, error) {
6263
queue := workqueue.NewTypedRateLimitingQueueWithConfig(workqueue.DefaultTypedControllerRateLimiter[string](), workqueue.TypedRateLimitingQueueConfig[string]{Name: controllerName})
6364

@@ -66,6 +67,9 @@ func NewController(
6667
providerConfig = rest.CopyConfig(providerConfig)
6768
providerConfig = rest.AddUserAgent(providerConfig, controllerName)
6869

70+
consumerConfig = rest.CopyConfig(consumerConfig)
71+
consumerConfig = rest.AddUserAgent(consumerConfig, controllerName)
72+
6973
providerClient, err := dynamicclient.NewForConfig(providerConfig)
7074
if err != nil {
7175
return nil, err
@@ -195,6 +199,9 @@ type controller struct {
195199

196200
providerNamespace string
197201

202+
// Channel to receive notifications when new namespaces are created
203+
namespaceCreationNotifyChan <-chan string
204+
198205
readReconciler
199206
}
200207

@@ -240,34 +247,24 @@ func (c *controller) enqueueConsumer(logger klog.Logger, obj interface{}) {
240247
runtime.HandleError(err)
241248
return
242249
}
250+
251+
c.enqueueConsumerByKey(logger, key)
252+
}
253+
254+
// enqueueConsumerByKey will enqueue the given namespaced object. This is split from enqueueConsumer
255+
// so we can accept channel requests from namespace creation watcher.
256+
func (c *controller) enqueueConsumerByKey(logger klog.Logger, key string) {
243257
ns, name, err := cache.SplitMetaNamespaceKey(key)
244258
if err != nil {
245259
runtime.HandleError(err)
246260
return
247261
}
248-
249262
if ns != "" { // Namespaced object.
250263
sn, err := c.serviceNamespaceInformer.Lister().APIServiceNamespaces(c.providerNamespace).Get(ns)
251264
if err != nil {
252265
if errors.IsNotFound(err) {
253-
// No namespace - create one.
254-
// TODO: This is not quite right place for this code. We should refactor this to be somewhere else.
255-
_, err := c.createServiceNamespace(context.TODO(), &kubebindv1alpha2.APIServiceNamespace{
256-
ObjectMeta: metav1.ObjectMeta{
257-
Name: ns,
258-
Namespace: c.providerNamespace,
259-
OwnerReferences: []metav1.OwnerReference{
260-
*metav1.NewControllerRef(c.apiServiceExport, kubebindv1alpha2.SchemeGroupVersion.WithKind("APIServiceExport")),
261-
},
262-
},
263-
})
264-
if err != nil && !errors.IsAlreadyExists(err) {
265-
runtime.HandleError(fmt.Errorf("failed to create APIServiceNamespace %q: %w", ns, err))
266-
return
267-
}
268-
// Requeue when the APIServiceNamespace is created.
269-
logger.V(2).Info("created APIServiceNamespace, requeueing", "namespace", ns)
270-
// Once APIServiceNamespace is created, the informer will pick it up and requeue the objects.
266+
// No APIServiceNamespace - means claimedresourcesnamespaces controller should create it
267+
logger.V(2).Info("no APIServiceNamespace found for claimed object. Will retry", "namespace", ns, "object", name)
271268
return
272269
}
273270
runtime.HandleError(fmt.Errorf("failed to get APIServiceNamespace %q: %w", ns, err))
@@ -285,7 +282,7 @@ func (c *controller) enqueueConsumer(logger klog.Logger, obj interface{}) {
285282
return
286283
}
287284

288-
logger.V(2).Info("queueing Unstructured", "key", key)
285+
logger.V(2).Info("queueing Unstructured", "key", key, "reason", "Consumer")
289286
c.queue.Add(key)
290287
}
291288

@@ -427,6 +424,14 @@ func (c *controller) Start(ctx context.Context, numThreads int) {
427424
},
428425
})
429426

427+
// Start a goroutine to listen for namespace creation notifications.
428+
go func() {
429+
for key := range c.namespaceCreationNotifyChan {
430+
logger.Info("received namespace creation notification", "namespace", key)
431+
c.enqueueConsumerByKey(logger, key)
432+
}
433+
}()
434+
430435
for i := 0; i < numThreads; i++ {
431436
go wait.UntilWithContext(ctx, c.startWorker, time.Second)
432437
}
@@ -450,7 +455,7 @@ func (c *controller) processNextWorkItem(ctx context.Context) bool {
450455

451456
logger := klog.FromContext(ctx).WithValues("key", key)
452457
ctx = klog.NewContext(ctx, logger)
453-
logger.V(2).Info("processing key")
458+
logger.Info("processing key")
454459

455460
// No matter what, tell the queue we're done with this key, to unblock
456461
// other workers.
@@ -474,12 +479,3 @@ func (c *controller) process(ctx context.Context, key string) error {
474479

475480
return c.reconcile(ctx, namespace, name)
476481
}
477-
478-
func (c *controller) createServiceNamespace(ctx context.Context, sns *kubebindv1alpha2.APIServiceNamespace) (*kubebindv1alpha2.APIServiceNamespace, error) {
479-
return c.providerBindClient.KubeBindV1alpha2().APIServiceNamespaces(sns.Namespace).Create(ctx, sns, metav1.CreateOptions{})
480-
}
481-
482-
// configMap test-namespace/test
483-
// goes into index - indexers.ServiceNamespaceByNamespace with "local name" ->
484-
485-
// providerNamespace/<api-service-namspaces.....>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# claimedresourcesnamespaces
2+
3+
When using claimed resources, it can be situations where resource does not yet exists in the consumer cluster OR
4+
we are operating in cluster scoped mode, and hence no resource namespace is created. In scenarios like this,
5+
`APIServiceNamespace` object is never creates, and hence backend will not create appropriate RBAC for the consumer to be able to
6+
access the resources.
7+
8+
It will watch same GVR resources, as claimed resources (hence the name of the controller), and will create `APIServiceNamespace` objects,
9+
if object is created in the consumer cluster.
10+
11+
We can't add this logic into `claimedresource` controller, as it never gets to healthy state until provider has right rbac configured and
12+
consumer can access the resources and hence start the informers. Alternative to this is create `APIServiceNamespace` objects outside reconile
13+
loop, but that would be against the controller pattern.

0 commit comments

Comments
 (0)