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

🌱 Remove reliance on controller-runtime scheme builder #9045

Merged
merged 2 commits into from
Aug 7, 2023
Merged

🌱 Remove reliance on controller-runtime scheme builder #9045

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0c9cdb
Remove reliance on controller-runtime scheme builder
JoelSpeed Jul 18, 2023
8b44611
Document schemebuilder pattern in implementers guide
JoelSpeed Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1beta1/cluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ type ClusterList struct {
}

func init() {
SchemeBuilder.Register(&Cluster{}, &ClusterList{})
objectTypes = append(objectTypes, &Cluster{}, &ClusterList{})
}

// FailureDomains is a slice of FailureDomains.
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/clusterclass_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,5 +706,5 @@ type ClusterClassList struct {
}

func init() {
SchemeBuilder.Register(&ClusterClass{}, &ClusterClassList{})
objectTypes = append(objectTypes, &ClusterClass{}, &ClusterClassList{})
}
17 changes: 13 additions & 4 deletions api/v1beta1/groupversion_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,26 @@ limitations under the License.
package v1beta1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/scheme"
)

var (
// GroupVersion is group version used to register these objects.
GroupVersion = schema.GroupVersion{Group: "cluster.x-k8s.io", Version: "v1beta1"}

// SchemeBuilder is used to add go types to the GroupVersionKind scheme.
SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
// schemeBuilder is used to add go types to the GroupVersionKind scheme.
schemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)

// AddToScheme adds the types in this group-version to the given scheme.
AddToScheme = SchemeBuilder.AddToScheme
AddToScheme = schemeBuilder.AddToScheme

objectTypes = []runtime.Object{}
)

func addKnownTypes(scheme *runtime.Scheme) error {
Copy link
Member

@sbueringer sbueringer Aug 3, 2023 •
edited

Choose a reason for hiding this comment

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

Q: Isn't this all the same as just:

func AddToScheme(s *runtime.Scheme) {
	s.AddKnownTypes(GroupVersion, objectTypes...)
	metav1.AddToGroupVersion(s, GroupVersion)
}

Would it be simpler to just define the func?

(seems a lot easier to understand to me, but maybe I'm missing something)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pattern seems to be common if you look at say core/v1 or any other API package, so I was just following the usual pattern.

In those other APIs the schemebuilder is exported though, perhaps we should be leaving ours exported for consistency?

Copy link
Member

@sbueringer sbueringer Aug 3, 2023 •
edited

Choose a reason for hiding this comment

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

No strong opinion from my side (@vincepri?)

I was just wondering if we have to go through the schemeBuilder if we only export the AddToScheme func as we could do that in a simpler way.

scheme.AddKnownTypes(GroupVersion, objectTypes...)
metav1.AddToGroupVersion(scheme, GroupVersion)
return nil
}
2 changes: 1 addition & 1 deletion api/v1beta1/machine_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,5 +307,5 @@ type MachineList struct {
}

func init() {
SchemeBuilder.Register(&Machine{}, &MachineList{})
objectTypes = append(objectTypes, &Machine{}, &MachineList{})
}
2 changes: 1 addition & 1 deletion api/v1beta1/machinedeployment_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ type MachineDeploymentList struct {
}

func init() {
SchemeBuilder.Register(&MachineDeployment{}, &MachineDeploymentList{})
objectTypes = append(objectTypes, &MachineDeployment{}, &MachineDeploymentList{})
}

// GetConditions returns the set of conditions for the machinedeployment.
Expand Down
5 changes: 3 additions & 2 deletions api/v1beta1/machinedeployment_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
. "github.com/onsi/gomega"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
Expand All @@ -44,8 +45,8 @@ func TestMachineDeploymentDefault(t *testing.T) {
},
}

scheme, err := SchemeBuilder.Build()
g.Expect(err).ToNot(HaveOccurred())
scheme := runtime.NewScheme()
g.Expect(AddToScheme(scheme)).To(Succeed())
defaulter := MachineDeploymentDefaulter(scheme)

t.Run("for MachineDeployment", defaultValidateTestCustomDefaulter(md, defaulter))
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/machinehealthcheck_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,5 @@ type MachineHealthCheckList struct {
}

func init() {
SchemeBuilder.Register(&MachineHealthCheck{}, &MachineHealthCheckList{})
objectTypes = append(objectTypes, &MachineHealthCheck{}, &MachineHealthCheckList{})
}
2 changes: 1 addition & 1 deletion api/v1beta1/machineset_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,5 +240,5 @@ type MachineSetList struct {
}

func init() {
SchemeBuilder.Register(&MachineSet{}, &MachineSetList{})
objectTypes = append(objectTypes, &MachineSet{}, &MachineSetList{})
}
5 changes: 3 additions & 2 deletions internal/test/builder/builders.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
Expand Down Expand Up @@ -1268,8 +1269,8 @@ func (m *MachineDeploymentBuilder) Build() *clusterv1.MachineDeployment {
}
}
if m.defaulter {
scheme, err := clusterv1.SchemeBuilder.Build()
if err != nil {
scheme := runtime.NewScheme()
if err := clusterv1.AddToScheme(scheme); err != nil {
panic(err)
}
ctx := admission.NewContextWithRequest(context.Background(), admission.Request{
Expand Down