Skip to content

Commit

Permalink
Add ClassNamespace index
Browse files Browse the repository at this point in the history
Signed-off-by: Danil-Grigorev <[email protected]>
  • Loading branch information
Danil-Grigorev committed Dec 4, 2024
1 parent e16aa76 commit 3639364
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 14 deletions.
23 changes: 23 additions & 0 deletions api/v1beta1/index/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (
const (
// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name.
ClusterClassNameField = "spec.topology.class"
// ClusterClassNamespaceField is used by the Cluster controller to index Clusters by ClusterClass namespace.
ClusterClassNamespaceField = "spec.topology.classNamespace"
)

// ByClusterClassName adds the cluster class name index to the
Expand All @@ -44,6 +46,18 @@ func ByClusterClassName(ctx context.Context, mgr ctrl.Manager) error {
return nil
}

// ByClusterClassNamespace adds the cluster class namespace index to the
// managers cache.
func ByClusterClassNamespace(ctx context.Context, mgr ctrl.Manager) error {
if err := mgr.GetCache().IndexField(ctx, &clusterv1.Cluster{},
ClusterClassNamespaceField,
ClusterByClusterClassClassNamespace,
); err != nil {
return errors.Wrap(err, "error setting index field")
}
return nil
}

// ClusterByClusterClassClassName contains the logic to index Clusters by ClusterClass name.
func ClusterByClusterClassClassName(o client.Object) []string {
cluster, ok := o.(*clusterv1.Cluster)
Expand All @@ -55,3 +69,12 @@ func ClusterByClusterClassClassName(o client.Object) []string {
}
return nil
}

// ClusterByClusterClassClassNamespace contains the logic to index Clusters by ClusterClass namespace.
func ClusterByClusterClassClassNamespace(o client.Object) []string {
cluster, ok := o.(*clusterv1.Cluster)
if !ok {
panic(fmt.Sprintf("Expected Cluster but got a %T", o))
}
return []string{cluster.GetClassKey().Namespace}
}
4 changes: 4 additions & 0 deletions api/v1beta1/index/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ func AddDefaultIndexes(ctx context.Context, mgr ctrl.Manager) error {
if err := ByClusterClassName(ctx, mgr); err != nil {
return err
}

if err := ByClusterClassNamespace(ctx, mgr); err != nil {
return err
}
}

if feature.Gates.Enabled(feature.MachinePool) {
Expand Down
10 changes: 6 additions & 4 deletions internal/controllers/topology/cluster/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/go-logr/logr"
"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/types"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -372,7 +373,10 @@ func (r *Reconciler) clusterClassToCluster(ctx context.Context, o client.Object)
if err := r.Client.List(
ctx,
clusterList,
client.MatchingFields{index.ClusterClassNameField: clusterClass.Name},
client.MatchingFieldsSelector{Selector: fields.AndSelectors(
fields.OneTermEqualSelector(index.ClusterClassNameField, clusterClass.Name),
fields.OneTermEqualSelector(index.ClusterClassNamespaceField, clusterClass.Namespace),
)},
); err != nil {
return nil
}
Expand All @@ -381,9 +385,7 @@ func (r *Reconciler) clusterClassToCluster(ctx context.Context, o client.Object)
// create a request for each of the clusters.
requests := []ctrl.Request{}
for i := range clusterList.Items {
if clusterList.Items[i].GetClassKey().Namespace == clusterClass.Namespace {
requests = append(requests, ctrl.Request{NamespacedName: util.ObjectKey(&clusterList.Items[i])})
}
requests = append(requests, ctrl.Request{NamespacedName: util.ObjectKey(&clusterList.Items[i])})
}
return requests
}
Expand Down
5 changes: 4 additions & 1 deletion internal/controllers/topology/cluster/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ func TestMain(m *testing.M) {
if err := index.AddDefaultIndexes(ctx, mgr); err != nil {
panic(fmt.Sprintf("unable to setup index: %v", err))
}
// Set up the ClusterClassName index explicitly here. This index is ordinarily created in
// Set up the ClusterClassName and ClusterClassNamespace index explicitly here. This index is ordinarily created in
// index.AddDefaultIndexes. That doesn't happen here because the ClusterClass feature flag is not set.
if err := index.ByClusterClassName(ctx, mgr); err != nil {
panic(fmt.Sprintf("unable to setup index: %v", err))
}
if err := index.ByClusterClassNamespace(ctx, mgr); err != nil {
panic(fmt.Sprintf("unable to setup index: %v", err))
}
}
setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) {
clusterCache, err := clustercache.SetupWithManager(ctx, mgr, clustercache.Options{
Expand Down
15 changes: 6 additions & 9 deletions internal/webhooks/clusterclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
Expand Down Expand Up @@ -379,20 +380,16 @@ func (webhook *ClusterClass) classNamesFromMPWorkerClass(w clusterv1.WorkersClas
func (webhook *ClusterClass) getClustersUsingClusterClass(ctx context.Context, clusterClass *clusterv1.ClusterClass) ([]clusterv1.Cluster, error) {
clusters := &clusterv1.ClusterList{}
err := webhook.Client.List(ctx, clusters,
client.MatchingFields{index.ClusterClassNameField: clusterClass.Name},
client.MatchingFieldsSelector{Selector: fields.AndSelectors(
fields.OneTermEqualSelector(index.ClusterClassNameField, clusterClass.Name),
fields.OneTermEqualSelector(index.ClusterClassNamespaceField, clusterClass.Namespace),
)},
)
if err != nil {
return nil, err
}

referencedClusters := []clusterv1.Cluster{}
for _, cluster := range clusters.Items {
if cluster.GetClassKey().Namespace == clusterClass.Namespace {
referencedClusters = append(referencedClusters, cluster)
}
}

return referencedClusters, nil
return clusters.Items, nil
}

func getClusterClassVariablesMapWithReverseIndex(clusterClassVariables []clusterv1.ClusterClassVariable) (map[string]*clusterv1.ClusterClassVariable, map[string]int) {
Expand Down
67 changes: 67 additions & 0 deletions internal/webhooks/clusterclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func TestClusterClassDefaultNamespaces(t *testing.T) {
fakeClient := fake.NewClientBuilder().
WithScheme(fakeScheme).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassClassNamespace).
Build()

// Create the webhook and add the fakeClient as its client.
Expand Down Expand Up @@ -1866,6 +1867,7 @@ func TestClusterClassValidation(t *testing.T) {
fakeClient := fake.NewClientBuilder().
WithScheme(fakeScheme).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassClassNamespace).
Build()

// Pin the compatibility version used in variable CEL validation to 1.29, so we don't have to continuously refactor
Expand Down Expand Up @@ -2513,6 +2515,7 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) {
WithScheme(fakeScheme).
WithObjects(tt.clusters...).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassClassNamespace).
Build()

// Create the webhook and add the fakeClient as its client.
Expand All @@ -2527,6 +2530,70 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) {
}
}

func TestGetClustersUsingClusterClass(t *testing.T) {
// NOTE: ClusterTopology feature flag is disabled by default, thus preventing to create or update ClusterClasses.
// Enabling the feature flag temporarily for this test.
utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)

topology := builder.ClusterTopology().WithClass("class1")

tests := []struct {
name string
clusterClass *clusterv1.ClusterClass
clusters []client.Object
expectErr bool
expectClusters []client.Object
}{
{
name: "ClusterClass should return clusters referencing it",
clusterClass: builder.ClusterClass("default", "class1").Build(),
clusters: []client.Object{
builder.Cluster("default", "cluster1").WithTopology(topology.Build()).Build(),
builder.Cluster("default", "cluster2").Build(),
builder.Cluster("other", "cluster2").WithTopology(topology.DeepCopy().WithClassNamespace("default").Build()).Build(),
builder.Cluster("other", "cluster3").WithTopology(topology.Build()).Build(),
},
expectClusters: []client.Object{
builder.Cluster("default", "cluster1").Build(),
builder.Cluster("other", "cluster2").Build(),
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

// Sets up the fakeClient for the test case.
fakeClient := fake.NewClientBuilder().
WithScheme(fakeScheme).
WithObjects(tt.clusters...).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassClassNamespace).
Build()

// Create the webhook and add the fakeClient as its client.
webhook := &ClusterClass{Client: fakeClient}
clusters, err := webhook.getClustersUsingClusterClass(ctx, tt.clusterClass)
if tt.expectErr {
g.Expect(err).To(HaveOccurred())
return
}
g.Expect(err).ToNot(HaveOccurred())

clusterKeys := []client.ObjectKey{}
for _, c := range clusters {
clusterKeys = append(clusterKeys, client.ObjectKeyFromObject(&c))
}
expectedKeys := []client.ObjectKey{}
for _, c := range tt.expectClusters {
expectedKeys = append(expectedKeys, client.ObjectKeyFromObject(c))
}
g.Expect(clusterKeys).To(Equal(expectedKeys))
})
}
}

func TestValidateAutoscalerAnnotationsForClusterClass(t *testing.T) {
tests := []struct {
name string
Expand Down

0 comments on commit 3639364

Please sign in to comment.