Skip to content

Commit b4368fc

Browse files
darkdocdarkdoc
committed
WIP: drop full client from reconcile object, and everywhere else
We need this change because we change the dynamic client to the runtime client. It also makes the logic somewhat clearer.
1 parent 06c6d6d commit b4368fc

File tree

5 files changed

+163
-198
lines changed

5 files changed

+163
-198
lines changed

‎internal/controller/argo.go‎

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,13 @@ import (
2525
"strings"
2626

2727
v1 "k8s.io/api/core/v1"
28+
kerrors "k8s.io/apimachinery/pkg/api/errors"
2829
"k8s.io/apimachinery/pkg/api/resource"
2930
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3031
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3132
"k8s.io/apimachinery/pkg/runtime"
3233
"k8s.io/apimachinery/pkg/runtime/schema"
3334
"k8s.io/apimachinery/pkg/types"
34-
"k8s.io/client-go/dynamic"
35-
"k8s.io/client-go/kubernetes"
3635
"sigs.k8s.io/controller-runtime/pkg/client"
3736
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3837

@@ -302,45 +301,81 @@ g, admin, role:admin`
302301
return &s
303302
}
304303

305-
func haveArgo(cl dynamic.Interface, name, namespace string) bool {
306-
gvr := schema.GroupVersionResource{Group: ArgoCDGroup, Version: ArgoCDVersion, Resource: ArgoCDResource}
307-
_, err := cl.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
308-
return err == nil
304+
func haveArgo(cl client.Client, name, namespace string) (bool, error) {
305+
// Using an unstructured object.
306+
unstructuredArgo := &unstructured.Unstructured{}
307+
unstructuredArgo.SetGroupVersionKind(schema.GroupVersionKind{
308+
Group: ArgoCDGroup,
309+
Kind: ArgoCDResource,
310+
Version: ArgoCDVersion,
311+
})
312+
err := cl.Get(context.Background(), client.ObjectKey{
313+
Namespace: namespace,
314+
Name: name,
315+
}, unstructuredArgo)
316+
317+
if err != nil {
318+
if kerrors.IsNotFound(err) {
319+
return false, nil
320+
}
321+
return false, err
322+
}
323+
return true, nil
309324
}
310325

311-
func createOrUpdateArgoCD(cl dynamic.Interface, fullClient kubernetes.Interface, name, namespace string) error {
326+
func createOrUpdateArgoCD(cl client.Client, name, namespace string) error {
312327
argo := newArgoCD(name, namespace)
313-
gvr := schema.GroupVersionResource{Group: ArgoCDGroup, Version: ArgoCDVersion, Resource: ArgoCDResource}
314-
315328
var err error
316-
// we skip this check if fullClient is explicitly nil for simpler testing
317-
if fullClient != nil {
318-
err = checkAPIVersion(fullClient, ArgoCDGroup, ArgoCDVersion)
319-
if err != nil {
320-
return fmt.Errorf("cannot find a sufficiently recent argocd crd version: %v", err)
321-
}
329+
330+
foundArgo, err := haveArgo(cl, name, namespace)
331+
if err != nil {
332+
return fmt.Errorf("cannot find a sufficiently recent argocd crd version: %v", err)
322333
}
323334

324-
if !haveArgo(cl, name, namespace) {
335+
if !foundArgo {
325336
// create it
326337
obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
327338
newArgo := &unstructured.Unstructured{Object: obj}
328-
_, err = cl.Resource(gvr).Namespace(namespace).Create(context.TODO(), newArgo, metav1.CreateOptions{})
339+
340+
// Using a unstructured object.
341+
342+
newArgo.SetGroupVersionKind(schema.GroupVersionKind{
343+
Group: ArgoCDGroup,
344+
Kind: ArgoCDResource,
345+
Version: ArgoCDVersion,
346+
})
347+
348+
err = cl.Create(context.Background(), newArgo)
329349
} else { // update it
330350
oldArgo, _ := getArgoCD(cl, name, namespace)
331351
argo.SetResourceVersion(oldArgo.GetResourceVersion())
332352
obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
333353
newArgo := &unstructured.Unstructured{Object: obj}
334-
335-
_, err = cl.Resource(gvr).Namespace(namespace).Update(context.TODO(), newArgo, metav1.UpdateOptions{})
354+
newArgo.SetGroupVersionKind(schema.GroupVersionKind{
355+
Group: ArgoCDGroup,
356+
Kind: ArgoCDResource,
357+
Version: ArgoCDVersion,
358+
})
359+
err = cl.Update(context.Background(), newArgo)
336360
}
337361
return err
338362
}
339363

340-
func getArgoCD(cl dynamic.Interface, name, namespace string) (*argooperator.ArgoCD, error) {
341-
gvr := schema.GroupVersionResource{Group: ArgoCDGroup, Version: ArgoCDVersion, Resource: ArgoCDResource}
364+
func getArgoCD(cl client.Client, name, namespace string) (*argooperator.ArgoCD, error) {
342365
argo := &argooperator.ArgoCD{}
343-
unstructuredArgo, err := cl.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
366+
367+
// Using an unstructured object.
368+
unstructuredArgo := &unstructured.Unstructured{}
369+
unstructuredArgo.SetGroupVersionKind(schema.GroupVersionKind{
370+
Group: ArgoCDGroup,
371+
Kind: ArgoCDResource,
372+
Version: ArgoCDVersion,
373+
})
374+
err := cl.Get(context.Background(), client.ObjectKey{
375+
Namespace: namespace,
376+
Name: name,
377+
}, unstructuredArgo)
378+
344379
if err != nil {
345380
return nil, err
346381
}

‎internal/controller/argo_test.go‎

Lines changed: 77 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,15 @@ import (
1818
kerrors "k8s.io/apimachinery/pkg/api/errors"
1919
"k8s.io/apimachinery/pkg/api/resource"
2020
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
21-
"k8s.io/apimachinery/pkg/runtime"
2221
"k8s.io/apimachinery/pkg/runtime/schema"
2322
"k8s.io/apimachinery/pkg/types"
24-
"k8s.io/client-go/dynamic"
25-
dynamicfake "k8s.io/client-go/dynamic/fake"
26-
"k8s.io/client-go/kubernetes/fake"
2723

2824
api "github.com/hybrid-cloud-patterns/patterns-operator/api/v1alpha1"
2925
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30-
"k8s.io/client-go/testing"
3126

3227
"sigs.k8s.io/controller-runtime/pkg/client"
3328

29+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3430
runtimefake "sigs.k8s.io/controller-runtime/pkg/client/fake"
3531
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
3632

@@ -839,120 +835,149 @@ var _ = Describe("NewArgoCD", func() {
839835
var _ = Describe("haveArgo", func() {
840836

841837
var (
842-
dynamicClient dynamic.Interface
843-
kubeClient *fake.Clientset
838+
fakeClient client.Client
844839

845-
gvr schema.GroupVersionResource
846840
name string
847841
namespace string
848842
)
849843

850844
BeforeEach(func() {
851-
gvr = schema.GroupVersionResource{Group: "argoproj.io", Version: "v1beta1", Resource: "argocds"}
852-
kubeClient = fake.NewSimpleClientset()
853-
dynamicClient = dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
854-
gvr: "ArgoCDList",
855-
})
856845
name = argoName
857846
namespace = argoNS
858847
})
859848

860849
Context("when the ArgoCD instance exists", func() {
861-
BeforeEach(func() {
862-
argoCD := &unstructured.Unstructured{
850+
It("should return true", func() {
851+
unstructuredArgo := &unstructured.Unstructured{
863852
Object: map[string]any{
864853
"apiVersion": "argoproj.io/v1beta1",
865854
"kind": "ArgoCD",
866855
"metadata": map[string]any{
867-
"name": name,
868-
"namespace": namespace,
856+
"name": name,
857+
"namespace": namespace,
858+
"resourceVersion": "1",
869859
},
870860
},
871861
}
872-
_, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(context.Background(), argoCD, metav1.CreateOptions{})
873-
Expect(err).ToNot(HaveOccurred())
874-
})
862+
unstructuredArgo.SetGroupVersionKind(schema.GroupVersionKind{
863+
Group: ArgoCDGroup,
864+
Kind: ArgoCDResource,
865+
Version: ArgoCDVersion,
866+
})
867+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
868+
WithRuntimeObjects(unstructuredArgo).Build()
875869

876-
It("should return true", func() {
877-
result := haveArgo(dynamicClient, name, namespace)
870+
result, err := haveArgo(fakeClient, name, namespace)
878871
Expect(result).To(BeTrue())
872+
Expect(err).ToNot(HaveOccurred())
873+
879874
})
880875
})
881876

882877
Context("when the ArgoCD instance does not exist", func() {
883878
It("should return false", func() {
884-
result := haveArgo(dynamicClient, name, namespace)
879+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
880+
WithRuntimeObjects().Build()
881+
result, err := haveArgo(fakeClient, name, namespace)
885882
Expect(result).To(BeFalse())
883+
Expect(err).ToNot(HaveOccurred())
884+
886885
})
887886
})
888887

889888
Context("when there is an error retrieving the ArgoCD instance", func() {
890-
BeforeEach(func() {
891-
kubeClient.PrependReactor("get", "argocds", func(testing.Action) (handled bool, ret runtime.Object, err error) {
892-
return true, nil, fmt.Errorf("get error")
893-
})
894-
})
895-
896889
It("should return false", func() {
897-
result := haveArgo(dynamicClient, name, namespace)
890+
fakeClient = fake.NewClientBuilder().WithInterceptorFuncs(
891+
interceptor.Funcs{Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
892+
return fmt.Errorf("get error")
893+
}},
894+
).WithScheme(testEnv.Scheme).Build()
895+
896+
result, err := haveArgo(fakeClient, name, namespace)
898897
Expect(result).To(BeFalse())
898+
Expect(err).To(HaveOccurred())
899+
899900
})
900901
})
901902
})
902903

903904
var _ = Describe("CreateOrUpdateArgoCD", func() {
904905
var (
905-
dynamicClient dynamic.Interface
906-
gvr schema.GroupVersionResource
907-
name string
908-
namespace string
906+
fakeClient client.Client
907+
name string
908+
namespace string
909909
)
910910

911911
BeforeEach(func() {
912-
gvr = schema.GroupVersionResource{Group: ArgoCDGroup, Version: ArgoCDVersion, Resource: ArgoCDResource}
913-
dynamicClient = dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
914-
gvr: "ArgoCDList",
915-
})
916912
name = argoName
917913
namespace = argoNS
918914
})
919915

920916
Context("when the ArgoCD instance does not exist", func() {
921917
It("should create a new ArgoCD instance", func() {
922-
err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace)
918+
919+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
920+
WithRuntimeObjects().Build()
921+
922+
err := createOrUpdateArgoCD(fakeClient, name, namespace)
923923
Expect(err).ToNot(HaveOccurred())
924924

925-
argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
925+
unstructuredArgo := &unstructured.Unstructured{}
926+
unstructuredArgo.SetGroupVersionKind(schema.GroupVersionKind{
927+
Group: ArgoCDGroup,
928+
Kind: ArgoCDResource,
929+
Version: ArgoCDVersion,
930+
})
931+
err = fakeClient.Get(context.Background(), client.ObjectKey{
932+
Namespace: namespace,
933+
Name: name,
934+
}, unstructuredArgo)
935+
926936
Expect(err).ToNot(HaveOccurred())
927-
Expect(argoCD.GetName()).To(Equal(name))
928-
Expect(argoCD.GetNamespace()).To(Equal(namespace))
937+
Expect(unstructuredArgo.GetName()).To(Equal(name))
938+
Expect(unstructuredArgo.GetNamespace()).To(Equal(namespace))
929939
})
930940
})
931941

932942
Context("when the ArgoCD instance exists", func() {
933-
BeforeEach(func() {
934-
argoCD := &unstructured.Unstructured{
943+
It("should update the existing ArgoCD instance", func() {
944+
945+
unstructuredArgo := &unstructured.Unstructured{
935946
Object: map[string]any{
936947
"apiVersion": "argoproj.io/v1beta1",
937948
"kind": "ArgoCD",
938949
"metadata": map[string]any{
939950
"name": name,
940951
"namespace": namespace,
941-
"resourceVersion": "1",
952+
"resourceVersion": "0",
942953
},
943954
},
944955
}
945-
_, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(context.TODO(), argoCD, metav1.CreateOptions{})
946-
Expect(err).ToNot(HaveOccurred())
947-
})
956+
unstructuredArgo.SetGroupVersionKind(schema.GroupVersionKind{
957+
Group: ArgoCDGroup,
958+
Kind: ArgoCDResource,
959+
Version: ArgoCDVersion,
960+
})
948961

949-
It("should update the existing ArgoCD instance", func() {
950-
err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace)
962+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
963+
WithRuntimeObjects(unstructuredArgo).Build()
964+
965+
err := createOrUpdateArgoCD(fakeClient, name, namespace)
951966
Expect(err).ToNot(HaveOccurred())
952967

953-
argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
968+
updatedUnstructuredArgo := &unstructured.Unstructured{}
969+
updatedUnstructuredArgo.SetGroupVersionKind(schema.GroupVersionKind{
970+
Group: ArgoCDGroup,
971+
Kind: ArgoCDResource,
972+
Version: ArgoCDVersion,
973+
})
974+
err = fakeClient.Get(context.Background(), client.ObjectKey{
975+
Namespace: namespace,
976+
Name: name,
977+
}, updatedUnstructuredArgo)
978+
954979
Expect(err).ToNot(HaveOccurred())
955-
Expect(argoCD.GetResourceVersion()).To(Equal("1")) // Ensure it has been updated
980+
Expect(updatedUnstructuredArgo.GetResourceVersion()).To(Equal("1")) // Ensure it has been updated
956981
})
957982
})
958983
})

‎internal/controller/kube.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ import (
2222

2323
routeclient "github.com/openshift/client-go/route/clientset/versioned"
2424
v1 "k8s.io/api/core/v1"
25-
"k8s.io/apimachinery/pkg/api/errors"
2625
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27-
"k8s.io/client-go/kubernetes"
2826

2927
"k8s.io/apimachinery/pkg/runtime/schema"
3028
"k8s.io/apimachinery/pkg/types"
@@ -86,28 +84,6 @@ func referSameObject(a, b *metav1.OwnerReference) bool {
8684
return aGV.Version == bGV.Version && aGV.Group == bGV.Group && a.Kind == b.Kind && a.Name == b.Name && a.UID == b.UID
8785
}
8886

89-
func checkAPIVersion(clientset kubernetes.Interface, group, version string) error {
90-
// Get the list of API groups available in the cluster
91-
apiGroups, err := clientset.Discovery().ServerGroups()
92-
if err != nil {
93-
return fmt.Errorf("failed to get API groups: %v", err)
94-
}
95-
96-
// Iterate through the API groups to find the specified group and version
97-
//nolint:gocritic // The range is so small that this is not worth changing
98-
for _, apiGroup := range apiGroups.Groups {
99-
if apiGroup.Name == group {
100-
for _, apiVersion := range apiGroup.Versions {
101-
if apiVersion.Version == version {
102-
return nil
103-
}
104-
}
105-
}
106-
}
107-
108-
return fmt.Errorf("API version %s/%s not available", group, version)
109-
}
110-
11187
func getRoute(routeClient routeclient.Interface, routeName, namespace string) (string, error) {
11288
route, err := routeClient.RouteV1().Routes(namespace).Get(context.Background(), routeName, metav1.GetOptions{})
11389
if err != nil {

0 commit comments

Comments
 (0)