Skip to content

Commit 65e9d46

Browse files
committed
Address review comments and clean up old code
Signed-off-by: Karol Szwaj <[email protected]> On-behalf-of: @SAP [email protected]
1 parent 857b19b commit 65e9d46

File tree

7 files changed

+71
-97
lines changed

7 files changed

+71
-97
lines changed

examples/apiexport/main.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,6 @@ import (
2323

2424
"github.com/spf13/pflag"
2525

26-
<<<<<<< HEAD
27-
=======
28-
"github.com/kcp-dev/multicluster-provider/apiexport"
29-
30-
mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder"
31-
mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
32-
mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile"
33-
34-
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
35-
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
36-
tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1"
37-
38-
>>>>>>> 5848f04 (update code structure and fix linters)
3926
corev1 "k8s.io/api/core/v1"
4027
apierrors "k8s.io/apimachinery/pkg/api/errors"
4128
"k8s.io/apimachinery/pkg/util/runtime"

examples/initializingworkspaces/main.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,32 @@ import (
2222
"os"
2323
"slices"
2424

25-
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
2625
"github.com/spf13/pflag"
2726
"go.uber.org/zap/zapcore"
28-
apierrors "k8s.io/apimachinery/pkg/api/errors"
29-
30-
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
31-
"github.com/kcp-dev/kcp/sdk/apis/tenancy/initialization"
32-
tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1"
33-
"github.com/kcp-dev/multicluster-provider/initializingworkspaces"
34-
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
35-
mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder"
36-
mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
37-
mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile"
3827

3928
corev1 "k8s.io/api/core/v1"
29+
apierrors "k8s.io/apimachinery/pkg/api/errors"
4030
"k8s.io/apimachinery/pkg/util/runtime"
4131
"k8s.io/client-go/kubernetes/scheme"
4232
"k8s.io/client-go/rest"
4333
ctrl "sigs.k8s.io/controller-runtime"
34+
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
4435
"sigs.k8s.io/controller-runtime/pkg/log"
4536
"sigs.k8s.io/controller-runtime/pkg/log/zap"
4637
"sigs.k8s.io/controller-runtime/pkg/manager"
4738
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
4839
"sigs.k8s.io/controller-runtime/pkg/reconcile"
40+
41+
mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder"
42+
mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
43+
mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile"
44+
45+
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
46+
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
47+
"github.com/kcp-dev/kcp/sdk/apis/tenancy/initialization"
48+
tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1"
49+
50+
"github.com/kcp-dev/multicluster-provider/initializingworkspaces"
4951
)
5052

5153
func init() {
@@ -162,7 +164,6 @@ func main() {
162164
return reconcile.Result{}, fmt.Errorf("failed to create configmap: %w", err)
163165
}
164166
log.Info("ConfigMap created successfully", "name", s.Name, "uuid", s.UID)
165-
166167
}
167168
// Remove the initializer from the logical cluster status
168169
// so that it won't be processed again.

initializingworkspaces/doc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ limitations under the License.
1616

1717
/*
1818
Package initializingworkspaces provides a [sigs.k8s.io/multicluster-runtime] provider implementation for interacting with
19-
initizializing virtual workspace exposed by a [kcp] instance. This provider can be used for writing controllers
19+
initializing virtual workspace exposed by a [kcp] instance. This provider can be used for writing controllers
2020
that reconcile Workspaces in the Initializing phase.
2121
2222
[kcp]: https://kcp.io

initializingworkspaces/provider.go

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,19 @@ import (
4949
var _ multicluster.Provider = &Provider{}
5050

5151
// Provider is a [sigs.k8s.io/multicluster-runtime/pkg/multicluster.Provider] that represents each [logical cluster]
52-
// (in the kcp sense) having specific initializer and exposed via initializingworkspaces virtual workspace endpoint as a cluster in the [sigs.k8s.io/multicluster-runtime] sense.
52+
// (in the kcp sense) having a specific initializer and exposed via the initializingworkspaces virtual workspace endpoint as a cluster in the [sigs.k8s.io/multicluster-runtime] sense.
5353
//
5454
// [logical cluster]: https://docs.kcp.io/kcp/latest/concepts/terminology/#logical-cluster
5555
type Provider struct {
5656
config *rest.Config
5757
scheme *runtime.Scheme
58-
cache cache.Cache
5958
wildcardCache mcpcache.WildcardCache
60-
object client.Object
6159
initializerName string
6260

6361
log logr.Logger
6462

6563
lock sync.RWMutex
66-
Clusters map[logicalcluster.Name]cluster.Cluster
64+
clusters map[logicalcluster.Name]cluster.Cluster
6765
cancelFns map[logicalcluster.Name]context.CancelFunc
6866
}
6967

@@ -75,9 +73,6 @@ type Options struct {
7573
// WildcardCache is the wildcard cache to use for the provider. If this is
7674
// nil, a new wildcard cache will be created for the given rest.Config.
7775
WildcardCache mcpcache.WildcardCache
78-
// ObjectToWatch is the object type that the provider watches. If this is nil,
79-
// it defaults to LogicalCluster.
80-
ObjectToWatch client.Object
8176

8277
// InitializerName is the name of the initializer to watch for in LogicalCluster.Status.Initializers
8378
InitializerName string
@@ -87,9 +82,6 @@ type Options struct {
8782
func New(cfg *rest.Config, options Options) (*Provider, error) {
8883
if options.Scheme == nil {
8984
options.Scheme = scheme.Scheme
90-
if err := kcpcorev1alpha1.AddToScheme(options.Scheme); err != nil {
91-
return nil, fmt.Errorf("failed to add kcp core scheme: %w", err)
92-
}
9385
}
9486

9587
if options.WildcardCache == nil {
@@ -102,10 +94,6 @@ func New(cfg *rest.Config, options Options) (*Provider, error) {
10294
}
10395
}
10496

105-
if options.ObjectToWatch == nil {
106-
options.ObjectToWatch = &kcpcorev1alpha1.LogicalCluster{}
107-
}
108-
10997
if options.InitializerName == "" {
11098
return nil, fmt.Errorf("initializer name cannot be empty")
11199
}
@@ -114,22 +102,20 @@ func New(cfg *rest.Config, options Options) (*Provider, error) {
114102
config: cfg,
115103
scheme: options.Scheme,
116104
wildcardCache: options.WildcardCache,
117-
object: options.ObjectToWatch,
118105
initializerName: options.InitializerName,
106+
log: log.Log.WithName("kcp-initializing-workspaces-provider"),
119107

120-
log: log.Log.WithName("kcp-initializing-workspaces-provider"),
121-
122-
Clusters: map[logicalcluster.Name]cluster.Cluster{},
108+
clusters: map[logicalcluster.Name]cluster.Cluster{},
123109
cancelFns: map[logicalcluster.Name]context.CancelFunc{},
124110
}, nil
125111
}
126112

127113
// Run starts the provider and blocks.
128114
func (p *Provider) Run(ctx context.Context, mgr mcmanager.Manager) error {
129115
g, ctx := errgroup.WithContext(ctx)
130-
inf, err := p.wildcardCache.GetInformer(ctx, p.object, cache.BlockUntilSynced(true))
116+
inf, err := p.wildcardCache.GetInformer(ctx, &kcpcorev1alpha1.LogicalCluster{}, cache.BlockUntilSynced(true))
131117
if err != nil {
132-
return fmt.Errorf("failed to get %T informer: %w", p.object, err)
118+
return fmt.Errorf("failed to get informer: %w", err)
133119
}
134120

135121
if _, err := inf.AddEventHandler(toolscache.ResourceEventHandlerFuncs{
@@ -162,7 +148,7 @@ func (p *Provider) Run(ctx context.Context, mgr mcmanager.Manager) error {
162148
p.log.Info("disengaging non-initializing workspace", "cluster", clusterName)
163149
cancel()
164150
delete(p.cancelFns, clusterName)
165-
delete(p.Clusters, clusterName)
151+
delete(p.clusters, clusterName)
166152
}
167153
p.lock.Unlock()
168154
} else {
@@ -174,14 +160,18 @@ func (p *Provider) Run(ctx context.Context, mgr mcmanager.Manager) error {
174160
}
175161

176162
g.Go(func() error {
177-
return p.wildcardCache.Start(ctx)
163+
err := p.wildcardCache.Start(ctx)
164+
if err != nil {
165+
return err
166+
}
167+
return nil
178168
})
179169

180170
syncCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
181171
defer cancel()
182172

183-
if _, err := p.wildcardCache.GetInformer(syncCtx, p.object, cache.BlockUntilSynced(true)); err != nil {
184-
return fmt.Errorf("failed to sync %T informer: %w", p.object, err)
173+
if _, err := p.wildcardCache.GetInformer(syncCtx, &kcpcorev1alpha1.LogicalCluster{}, cache.BlockUntilSynced(true)); err != nil {
174+
return fmt.Errorf("failed to sync informer: %w", err)
185175
}
186176

187177
return g.Wait()
@@ -207,35 +197,37 @@ func (p *Provider) handleLogicalClusterEvent(ctx context.Context, obj any, mgr m
207197
// If our initializer is not present, we need to disengage the cluster if it exists
208198
if !hasInitializer {
209199
p.lock.Lock()
200+
defer p.lock.Unlock()
210201
cancel, ok := p.cancelFns[clusterName]
211202
if ok {
212203
p.log.Info("disengaging non-initializing workspace", "cluster", clusterName)
213204
cancel()
214205
delete(p.cancelFns, clusterName)
215-
delete(p.Clusters, clusterName)
206+
delete(p.clusters, clusterName)
216207
}
217-
p.lock.Unlock()
218208
return
219209
}
220210

221-
p.log.Info("LogicalCluster added", "object", clusterName)
222211
// fast path: cluster exists already, there is nothing to do.
223212
p.lock.RLock()
224-
if _, ok := p.Clusters[clusterName]; ok {
213+
if _, ok := p.clusters[clusterName]; ok {
225214
p.lock.RUnlock()
226215
return
227216
}
228217
p.lock.RUnlock()
229218

230219
// slow path: take write lock to add a new cluster (unless it appeared in the meantime).
231220
p.lock.Lock()
232-
if _, ok := p.Clusters[clusterName]; ok {
221+
if _, ok := p.clusters[clusterName]; ok {
233222
p.lock.Unlock()
234223
return
235224
}
236225

226+
p.log.Info("LogicalCluster added", "object", clusterName)
237227
// create new specific cluster with correct host endpoint for fetching specific logical cluster.
238-
ctx, cancel := context.WithCancel(ctx)
228+
clusterCtx, cancel := context.WithCancel(ctx)
229+
defer cancel()
230+
239231
cfg := rest.CopyConfig(p.config)
240232
host := cfg.Host
241233
host = strings.TrimSuffix(host, "/clusters/*")
@@ -248,20 +240,17 @@ func (p *Provider) handleLogicalClusterEvent(ctx context.Context, obj any, mgr m
248240
p.lock.Unlock()
249241
return
250242
}
251-
p.Clusters[clusterName] = cl
243+
p.clusters[clusterName] = cl
252244
p.cancelFns[clusterName] = cancel
253245
p.lock.Unlock()
254246

255-
_, syncCancel := context.WithTimeout(ctx, 10*time.Second)
256-
defer syncCancel()
257-
258247
p.log.Info("engaging cluster", "cluster", clusterName)
259-
if err := mgr.Engage(ctx, clusterName.String(), cl); err != nil {
248+
if err := mgr.Engage(clusterCtx, clusterName.String(), cl); err != nil {
260249
p.log.Error(err, "failed to engage cluster", "cluster", clusterName)
261250
p.lock.Lock()
262251
cancel()
263-
if p.Clusters[clusterName] == cl {
264-
delete(p.Clusters, clusterName)
252+
if p.clusters[clusterName] == cl {
253+
delete(p.clusters, clusterName)
265254
delete(p.cancelFns, clusterName)
266255
}
267256
p.lock.Unlock()
@@ -273,7 +262,7 @@ func (p *Provider) handleLogicalClusterEvent(ctx context.Context, obj any, mgr m
273262
func (p *Provider) Get(_ context.Context, name string) (cluster.Cluster, error) {
274263
p.lock.RLock()
275264
defer p.lock.RUnlock()
276-
if cl, ok := p.Clusters[logicalcluster.Name(name)]; ok {
265+
if cl, ok := p.clusters[logicalcluster.Name(name)]; ok {
277266
return cl, nil
278267
}
279268

@@ -282,5 +271,5 @@ func (p *Provider) Get(_ context.Context, name string) (cluster.Cluster, error)
282271

283272
// IndexField indexes the given object by the given field on all engaged clusters, current and future.
284273
func (p *Provider) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
285-
return p.cache.IndexField(ctx, obj, field, extractValue)
274+
return p.wildcardCache.IndexField(ctx, obj, field, extractValue)
286275
}

internal/cache/cache.go

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,32 @@ import (
3030
"github.com/kcp-dev/logicalcluster/v3"
3131
)
3232

33-
var _ cache.Cache = &ScopedCache{}
33+
var _ cache.Cache = &scopedCache{}
3434

35-
// ScopedCache is a Cache that operates on a specific cluster.
36-
type ScopedCache struct {
37-
Base WildcardCache
38-
ClusterName logicalcluster.Name
35+
// scopedCache is a Cache that operates on a specific cluster.
36+
type scopedCache struct {
37+
base WildcardCache
38+
clusterName logicalcluster.Name
3939
}
4040

4141
// Start starts the cache.
42-
func (c *ScopedCache) Start(ctx context.Context) error {
42+
func (c *scopedCache) Start(ctx context.Context) error {
4343
return errors.New("scoped cache cannot be started")
4444
}
4545

4646
// WaitForCacheSync waits for the cache to be synced.
47-
func (c *ScopedCache) WaitForCacheSync(ctx context.Context) bool {
48-
return c.Base.WaitForCacheSync(ctx)
47+
func (c *scopedCache) WaitForCacheSync(ctx context.Context) bool {
48+
return c.base.WaitForCacheSync(ctx)
4949
}
5050

5151
// IndexField indexes a field in the cache.
52-
func (c *ScopedCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
53-
return c.Base.IndexField(ctx, obj, field, extractValue)
52+
func (c *scopedCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
53+
return c.base.IndexField(ctx, obj, field, extractValue)
5454
}
5555

5656
// Get returns a single object from the cache.
57-
func (c *ScopedCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
58-
inf, gvk, scope, err := c.Base.GetSharedInformer(obj)
57+
func (c *scopedCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
58+
inf, gvk, scope, err := c.base.GetSharedInformer(obj)
5959
if err != nil {
6060
return fmt.Errorf("failed to get informer for %T %s: %w", obj, obj.GetObjectKind().GroupVersionKind(), err)
6161
}
@@ -65,15 +65,15 @@ func (c *ScopedCache) Get(ctx context.Context, key client.ObjectKey, obj client.
6565
groupVersionKind: gvk,
6666
scopeName: scope,
6767
disableDeepCopy: false,
68-
clusterName: c.ClusterName,
68+
clusterName: c.clusterName,
6969
}
7070

7171
return cr.Get(ctx, key, obj, opts...)
7272
}
7373

7474
// List returns a list of objects from the cache.
75-
func (c *ScopedCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
76-
inf, gvk, scope, err := c.Base.GetSharedInformer(list)
75+
func (c *scopedCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
76+
inf, gvk, scope, err := c.base.GetSharedInformer(list)
7777
if err != nil {
7878
return fmt.Errorf("failed to get informer for %T %s: %w", list, list.GetObjectKind().GroupVersionKind(), err)
7979
}
@@ -83,32 +83,32 @@ func (c *ScopedCache) List(ctx context.Context, list client.ObjectList, opts ...
8383
groupVersionKind: gvk,
8484
scopeName: scope,
8585
disableDeepCopy: false,
86-
clusterName: c.ClusterName,
86+
clusterName: c.clusterName,
8787
}
8888

8989
return cr.List(ctx, list, opts...)
9090
}
9191

9292
// GetInformer returns an informer for the given object kind.
93-
func (c *ScopedCache) GetInformer(ctx context.Context, obj client.Object, opts ...cache.InformerGetOption) (cache.Informer, error) {
94-
inf, err := c.Base.GetInformer(ctx, obj, opts...)
93+
func (c *scopedCache) GetInformer(ctx context.Context, obj client.Object, opts ...cache.InformerGetOption) (cache.Informer, error) {
94+
inf, err := c.base.GetInformer(ctx, obj, opts...)
9595
if err != nil {
9696
return nil, err
9797
}
98-
return &scopedInformer{clusterName: c.ClusterName, Informer: inf}, nil
98+
return &scopedInformer{clusterName: c.clusterName, Informer: inf}, nil
9999
}
100100

101101
// GetInformerForKind returns an informer for the given GroupVersionKind.
102-
func (c *ScopedCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...cache.InformerGetOption) (cache.Informer, error) {
103-
inf, err := c.Base.GetInformerForKind(ctx, gvk, opts...)
102+
func (c *scopedCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, opts ...cache.InformerGetOption) (cache.Informer, error) {
103+
inf, err := c.base.GetInformerForKind(ctx, gvk, opts...)
104104
if err != nil {
105105
return nil, err
106106
}
107-
return &scopedInformer{clusterName: c.ClusterName, Informer: inf}, nil
107+
return &scopedInformer{clusterName: c.clusterName, Informer: inf}, nil
108108
}
109109

110110
// RemoveInformer removes an informer from the cache.
111-
func (c *ScopedCache) RemoveInformer(ctx context.Context, obj client.Object) error {
111+
func (c *scopedCache) RemoveInformer(ctx context.Context, obj client.Object) error {
112112
return errors.New("informer cannot be removed from scoped cache")
113113
}
114114

0 commit comments

Comments
 (0)