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.

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

✨ create index for cluster.spec.topology.class #4967

Conversation

ykakarap
Copy link
Contributor

What this PR does / why we need it:
This PR adds an index for the cluster.spec.topology.class.
This is to enable efficiently finding clusters that are stamped from a given ClusterClass.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #4931

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 19, 2021
@ykakarap ykakarap force-pushed the clusterclass_cluster_topology_index branch from 8d23f74 to fda6322 Compare July 19, 2021 23:11
@ykakarap
Copy link
Contributor Author

/assign @fabriziopandini

api/v1alpha4/common_types.go Outdated Show resolved Hide resolved
controllers/clusterclassutil/indexer.go Outdated Show resolved Hide resolved
controllers/clusterclassutil/indexer_test.go Outdated Show resolved Hide resolved
controllers/clusterclassutil/indexer.go Outdated Show resolved Hide resolved
@fabriziopandini
Copy link
Member

fabriziopandini commented Jul 20, 2021

Changes lgtm (waiting for pending comments)
It will be great if we can wait for #4965 to be merged, so we can actually use the new index for the controller watch

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2021
@fabriziopandini
Copy link
Member

#4965 is merged, so it now possible to use the new index in the ClusterTopology controller!

@ykakarap
Copy link
Contributor Author

Will need to rebase this after #5004 is merged.

@sbueringer
Copy link
Member

@ykakarap Not sure exactly how the rebase will affect this but would be (probably ?) also good to make sure the index is used in our tests (please note #5030 refactors the envtest setup)

@ykakarap ykakarap force-pushed the clusterclass_cluster_topology_index branch from fda6322 to 253d4e9 Compare August 10, 2021 19:06
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2021
@ykakarap
Copy link
Contributor Author

/retest

@ykakarap ykakarap force-pushed the clusterclass_cluster_topology_index branch from 253d4e9 to f873fd0 Compare August 10, 2021 19:33
@ykakarap
Copy link
Contributor Author

/retest

Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2021
@k8s-ci-robot k8s-ci-robot merged commit cbaa42d into kubernetes-sigs:master Aug 11, 2021
@k8s-ci-robot k8s-ci-robot added this to the v0.4 milestone Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a custom cache index for Cluster.Topology.Class
6 participants