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.

Sign up for GitHub

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

Jump to bottom

🌱 Deprecate ClusterCacheTracker #11312

Merged
merged 1 commit into from
Oct 22, 2024
+20 −3
Merged

🌱 Deprecate ClusterCacheTracker #11312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions controllers/remote/cluster_cache_reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (

// ClusterCacheReconciler is responsible for stopping remote cluster caches when
// the cluster for the remote cache is being deleted.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.ClusterCache instead.
Copy link
Member Author

@sbueringer sbueringer Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think it makes sense to give folks one release time to migrate away (ClusterCacheTracker would be removed with the v1.10 (April 2025) release).

Folks can always just copy the old ClusterCacheTracker if they want to continue to use it.

Copy link
Member

@fabriziopandini fabriziopandini Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

works for me, should we change the PR type to make this surface in the release notes?

Copy link
Member Author

Choose a reason for hiding this comment

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

To what do you want to change it?

(Just adding the deprecation itself is not a breaking change)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a note here: #11092 (comment)

type ClusterCacheReconciler struct {
Client client.Client
Tracker *ClusterCacheTracker
Expand Down
6 changes: 6 additions & 0 deletions controllers/remote/cluster_cache_tracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ const (
var ErrClusterLocked = errors.New("cluster is locked already")

// ClusterCacheTracker manages client caches for workload clusters.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.ClusterCache instead.
type ClusterCacheTracker struct {
log logr.Logger

Expand Down Expand Up @@ -108,6 +110,8 @@ type ClusterCacheTracker struct {

// ClusterCacheTrackerOptions defines options to configure
// a ClusterCacheTracker.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.ClusterCache instead.
type ClusterCacheTrackerOptions struct {
// SecretCachingClient is a client which caches secrets.
// If set it will be used to read the kubeconfig secret.
Expand Down Expand Up @@ -172,6 +176,8 @@ func setDefaultOptions(opts *ClusterCacheTrackerOptions) {
}

// NewClusterCacheTracker creates a new ClusterCacheTracker.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.SetupWithManager instead.
func NewClusterCacheTracker(manager ctrl.Manager, options ClusterCacheTrackerOptions) (*ClusterCacheTracker, error) {
setDefaultOptions(&options)

Expand Down
2 changes: 2 additions & 0 deletions controllers/remote/cluster_cache_tracker_fake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
)

// NewTestClusterCacheTracker creates a new fake ClusterCacheTracker that can be used by unit tests with fake client.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.NewTestClusterCacheTracker instead.
func NewTestClusterCacheTracker(log logr.Logger, cl client.Client, remoteClient client.Client, scheme *runtime.Scheme, objKey client.ObjectKey, watchObjects ...string) *ClusterCacheTracker {
testCacheTracker := &ClusterCacheTracker{
log: log,
Expand Down
4 changes: 4 additions & 0 deletions controllers/remote/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ import (
)

// Index is a helper to model the info passed to cache.IndexField.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.CacheOptionsIndex instead.
type Index struct {
Object client.Object
Field string
ExtractValue client.IndexerFunc
}

// NodeProviderIDIndex is used to index Nodes by ProviderID.
//
// Deprecated: This will be removed in Cluster API v1.10, use clustercache.NodeProviderIDIndex instead.
var NodeProviderIDIndex = Index{
Object: &corev1.Node{},
Field: index.NodeProviderIDField,
Expand Down
3 changes: 0 additions & 3 deletions controlplane/kubeadm/internal/controllers/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
r.ssaCache = ssa.NewCache()

if r.managementCluster == nil {
if r.ClusterCache == nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a minor cleanup, this check is never used because we would already panic above now.

I have a follow-up to introduce proper input validation for all SetupWithManager funcs

return errors.New("cluster cache tracker is nil, cannot create the internal management cluster resource")
}
r.managementCluster = &internal.Management{
Client: r.Client,
SecretCachingClient: r.SecretCachingClient,
Expand Down
6 changes: 6 additions & 0 deletions docs/book/src/developer/providers/migrations/v1.8-to-v1.9.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ maintainers of providers and consumers of our Go API.
### Suggested changes for providers

- The Errors package was created when capi provider implementation was running as machineActuators that needed to vendor core capi to function. There is no usage recommendations today and its value is questionable since we moved to CRDs that inter-operate mostly via conditions. Instead we plan to drop the dedicated semantic for terminal failure and keep improving Machine lifecycle signal through conditions. Therefore the Errors package [has been deprecated in v1.8](https://github.com/kubernetes-sigs/cluster-api/issues/10784). It's recommended to remove any usage of the currently exported variables.
- The `ClusterCacheTracker` component has been deprecated, please use the new `ClusterCache` instead. For more context and examples for
how to use it, see [PR: Introduce new ClusterCache](https://github.com/kubernetes-sigs/cluster-api/pull/11247) and the corresponding
[issue](https://github.com/kubernetes-sigs/cluster-api/issues/11272). Some notes:
- The `DisabledFor` option (previously `ClientUncachedObjects`) is not defaulted to `&corev1.ConfigMap` & `&corev1.Secret` anymore,
thus it's now necessary to explicitly set `DisabledFor` to avoid caching ConfigMaps and Secrets.
- `SecretClient` and `UserAgent` are now mandatory options, please take a look at the corresponding godoc.