Skip to content

Commit fe693ff

Browse files
darkdocdarkdoc
committed
WIP: drop full client from reconcile object, and everywhere else
1 parent 0359c7f commit fe693ff

File tree

70 files changed

+212
-1595
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+212
-1595
lines changed

‎internal/controller/acm.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,28 @@ import (
2222
"fmt"
2323
"log"
2424

25+
corev1 "k8s.io/api/core/v1"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/labels"
2628
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
2730
)
2831

2932
func haveACMHub(r *PatternReconciler) bool {
3033
gvrMCH := schema.GroupVersionResource{Group: "operator.open-cluster-management.io", Version: "v1", Resource: "multiclusterhubs"}
3134

32-
serverNamespace := ""
35+
labelSelector, err := labels.Parse(fmt.Sprintf("%v = %v", "ocm-configmap-type", "image-manifest"))
3336

34-
cms, err := r.fullClient.CoreV1().ConfigMaps("").List(context.TODO(), metav1.ListOptions{
35-
LabelSelector: fmt.Sprintf("%v = %v", "ocm-configmap-type", "image-manifest"),
36-
})
37-
if (err != nil || len(cms.Items) == 0) && serverNamespace != "" {
38-
cms, err = r.fullClient.CoreV1().ConfigMaps(serverNamespace).List(context.TODO(), metav1.ListOptions{
39-
LabelSelector: fmt.Sprintf("%v = %v", "ocm-configmap-type", "image-manifest"),
40-
})
41-
}
42-
if err != nil || len(cms.Items) == 0 {
43-
cms, err = r.fullClient.CoreV1().ConfigMaps("open-cluster-management").List(context.TODO(), metav1.ListOptions{
44-
LabelSelector: fmt.Sprintf("%v = %v", "ocm-configmap-type", "image-manifest"),
45-
})
37+
if err != nil {
38+
log.Printf("config map error: %s\n", err.Error())
39+
return false
4640
}
41+
42+
cms := corev1.ConfigMapList{}
43+
err = r.List(context.Background(), &cms, &client.ListOptions{
44+
LabelSelector: labelSelector,
45+
})
46+
4747
if err != nil {
4848
log.Printf("config map error: %s\n", err.Error())
4949
return false

‎internal/controller/acm_test.go‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import (
88
"k8s.io/apimachinery/pkg/runtime"
99
"k8s.io/apimachinery/pkg/runtime/schema"
1010
dynamicfake "k8s.io/client-go/dynamic/fake"
11-
"k8s.io/client-go/kubernetes/fake"
1211
"k8s.io/client-go/testing"
12+
"sigs.k8s.io/controller-runtime/pkg/client"
13+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
14+
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
1315

1416
v1 "k8s.io/api/core/v1"
1517
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -21,24 +23,18 @@ import (
2123
var _ = Describe("HaveACMHub", func() {
2224
var (
2325
patternReconciler *PatternReconciler
24-
kubeClient *fake.Clientset
2526
dynamicClient *dynamicfake.FakeDynamicClient
2627
gvrMCH schema.GroupVersionResource
28+
fakeClient client.Client
2729
)
2830

2931
BeforeEach(func() {
30-
kubeClient = fake.NewSimpleClientset()
3132
gvrMCH = schema.GroupVersionResource{Group: "operator.open-cluster-management.io", Version: "v1", Resource: "multiclusterhubs"}
3233

3334
dynamicClient = dynamicfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
3435
gvrMCH: "MultiClusterHubList",
3536
})
3637

37-
patternReconciler = &PatternReconciler{
38-
fullClient: kubeClient,
39-
dynamicClient: dynamicClient,
40-
}
41-
4238
})
4339

4440
Context("when the ACM Hub exists", func() {
@@ -52,8 +48,13 @@ var _ = Describe("HaveACMHub", func() {
5248
},
5349
},
5450
}
55-
_, err := kubeClient.CoreV1().ConfigMaps("default").Create(context.Background(), configMap, metav1.CreateOptions{})
56-
Expect(err).ToNot(HaveOccurred())
51+
52+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
53+
WithRuntimeObjects(configMap).Build()
54+
patternReconciler = &PatternReconciler{
55+
Client: fakeClient,
56+
dynamicClient: dynamicClient,
57+
}
5758

5859
hub := &unstructured.Unstructured{
5960
Object: map[string]any{
@@ -65,7 +66,7 @@ var _ = Describe("HaveACMHub", func() {
6566
},
6667
},
6768
}
68-
_, err = dynamicClient.Resource(gvrMCH).Namespace("default").Create(context.Background(), hub, metav1.CreateOptions{})
69+
_, err := dynamicClient.Resource(gvrMCH).Namespace("default").Create(context.Background(), hub, metav1.CreateOptions{})
6970
Expect(err).ToNot(HaveOccurred())
7071
})
7172

@@ -77,16 +78,28 @@ var _ = Describe("HaveACMHub", func() {
7778

7879
Context("when the ACM Hub does not exist", func() {
7980
It("should return false", func() {
81+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
82+
WithRuntimeObjects().Build()
83+
patternReconciler = &PatternReconciler{
84+
Client: fakeClient,
85+
dynamicClient: dynamicClient,
86+
}
87+
8088
result := haveACMHub(patternReconciler)
8189
Expect(result).To(BeFalse())
8290
})
8391
})
8492

8593
Context("when there is an error listing ConfigMaps", func() {
8694
BeforeEach(func() {
87-
kubeClient.PrependReactor("list", "configmaps", func(testing.Action) (handled bool, ret runtime.Object, err error) {
88-
return true, nil, fmt.Errorf("config map error")
89-
})
95+
fakeClient = fake.NewClientBuilder().WithInterceptorFuncs(
96+
interceptor.Funcs{List: func(ctx context.Context, client client.WithWatch, obj client.ObjectList, opts ...client.ListOption) error {
97+
return fmt.Errorf("list error")
98+
}}).WithScheme(testEnv.Scheme).Build()
99+
patternReconciler = &PatternReconciler{
100+
Client: fakeClient,
101+
dynamicClient: dynamicClient,
102+
}
90103
})
91104

92105
It("should return false and log the error", func() {
@@ -106,12 +119,16 @@ var _ = Describe("HaveACMHub", func() {
106119
},
107120
},
108121
}
109-
_, err := kubeClient.CoreV1().ConfigMaps("default").Create(context.Background(), configMap, metav1.CreateOptions{})
110-
Expect(err).ToNot(HaveOccurred())
122+
fakeClient = fake.NewClientBuilder().WithScheme(testEnv.Scheme).
123+
WithRuntimeObjects(configMap).Build()
111124

112125
dynamicClient.PrependReactor("list", "multiclusterhubs", func(testing.Action) (handled bool, ret runtime.Object, err error) {
113126
return true, nil, fmt.Errorf("multiclusterhub error")
114127
})
128+
patternReconciler = &PatternReconciler{
129+
Client: fakeClient,
130+
dynamicClient: dynamicClient,
131+
}
115132
})
116133

117134
It("should return false and log the error", func() {

‎internal/controller/checkout.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"path/filepath"
2727

2828
stdssh "golang.org/x/crypto/ssh"
29-
"k8s.io/client-go/kubernetes"
29+
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
3030

3131
"github.com/go-git/go-git/v5"
3232
"github.com/go-git/go-git/v5/plumbing"
@@ -77,8 +77,8 @@ func (g *GitOperationsImpl) CloneRepository(directory string, isBare bool, optio
7777
}
7878

7979
// https://github.com/go-git/go-git/blob/master/_examples/commit/main.go
80-
func checkout(fullClient kubernetes.Interface, gitOps GitOperations, url, directory, commit string, secret map[string][]byte) error {
81-
if err := cloneRepo(fullClient, gitOps, url, directory, secret); err != nil {
80+
func checkout(cl ctrlclient.Client, gitOps GitOperations, url, directory, commit string, secret map[string][]byte) error {
81+
if err := cloneRepo(cl, gitOps, url, directory, secret); err != nil {
8282
return err
8383
}
8484

@@ -87,7 +87,7 @@ func checkout(fullClient kubernetes.Interface, gitOps GitOperations, url, direct
8787
return nil
8888
}
8989

90-
if err := checkoutRevision(fullClient, gitOps, url, directory, commit, secret); err != nil {
90+
if err := checkoutRevision(cl, gitOps, url, directory, commit, secret); err != nil {
9191
return err
9292
}
9393

@@ -163,9 +163,9 @@ func getCommitFromTarget(repo *git.Repository, name string) (plumbing.Hash, erro
163163
return plumbing.ZeroHash, fmt.Errorf("unknown target %q", name)
164164
}
165165

166-
func checkoutRevision(fullClient kubernetes.Interface, gitOps GitOperations, url, directory, commit string, secret map[string][]byte) error {
166+
func checkoutRevision(cl ctrlclient.Client, gitOps GitOperations, url, directory, commit string, secret map[string][]byte) error {
167167
customClient := &nethttp.Client{
168-
Transport: getHTTPSTransport(fullClient),
168+
Transport: getHTTPSTransport(cl),
169169
}
170170
// Override http(s) default protocol to use our custom client
171171
client.InstallProtocol("https", http.NewClient(customClient))
@@ -221,9 +221,9 @@ func checkoutRevision(fullClient kubernetes.Interface, gitOps GitOperations, url
221221
return err
222222
}
223223

224-
func cloneRepo(fullClient kubernetes.Interface, gitOps GitOperations, url, directory string, secret map[string][]byte) error {
224+
func cloneRepo(cl ctrlclient.Client, gitOps GitOperations, url, directory string, secret map[string][]byte) error {
225225
customClient := &nethttp.Client{
226-
Transport: getHTTPSTransport(fullClient),
226+
Transport: getHTTPSTransport(cl),
227227
}
228228
// Override http(s) default protocol to use our custom client
229229
client.InstallProtocol("https", http.NewClient(customClient))

‎internal/controller/gitea-util.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,21 @@ import (
2121
"net/http"
2222

2323
"code.gitea.io/sdk/gitea"
24-
"k8s.io/client-go/kubernetes"
24+
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
2525
)
2626

2727
type GiteaOperations interface {
28-
MigrateGiteaRepo(fullClient kubernetes.Interface, username, password, upstreamURL, giteaServerRoute string) (success bool, repositoryURL string, err error)
28+
MigrateGiteaRepo(cl ctrlclient.Client, username, password, upstreamURL, giteaServerRoute string) (success bool, repositoryURL string, err error)
2929
}
3030

3131
type GiteaOperationsImpl struct{}
3232

3333
// Function that creates a mirror repo in Gitea
3434
func (g *GiteaOperationsImpl) MigrateGiteaRepo(
35-
fullClient kubernetes.Interface, username, password, upstreamURL, giteaServerRoute string) (success bool, repositoryURL string, err error) {
35+
cl ctrlclient.Client, username, password, upstreamURL, giteaServerRoute string) (success bool, repositoryURL string, err error) {
3636
option := gitea.SetBasicAuth(username, password)
3737
httpClient := &http.Client{
38-
Transport: getHTTPSTransport(fullClient),
38+
Transport: getHTTPSTransport(cl),
3939
}
4040

4141
giteaClient, err := gitea.NewClient(giteaServerRoute, option, gitea.SetHTTPClient(httpClient))

‎internal/controller/gitea-util_test.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"net/http/httptest"
77

88
gomock "go.uber.org/mock/gomock"
9-
"k8s.io/client-go/kubernetes/fake"
9+
"sigs.k8s.io/controller-runtime/pkg/client"
1010

1111
. "github.com/onsi/ginkgo/v2"
1212
. "github.com/onsi/gomega"
@@ -15,14 +15,14 @@ import (
1515
var _ = Describe("MigrateGiteaRepo", func() {
1616
var (
1717
mockCtrl *gomock.Controller
18-
mockKubeClient *fake.Clientset
1918
giteaServer *httptest.Server
2019
giteaServerRoute string
2120
giteaOperations GiteaOperations
2221
username string
2322
password string
2423
upstreamURL string
2524
repoName string
25+
fakeClient client.Client
2626
)
2727

2828
const (
@@ -32,7 +32,6 @@ var _ = Describe("MigrateGiteaRepo", func() {
3232

3333
BeforeEach(func() {
3434
mockCtrl = gomock.NewController(GinkgoT())
35-
mockKubeClient = fake.NewSimpleClientset()
3635
giteaOperations = &GiteaOperationsImpl{}
3736
username = "user"
3837
password = "pass"
@@ -66,7 +65,7 @@ var _ = Describe("MigrateGiteaRepo", func() {
6665

6766
Context("when the repository does not exist", func() {
6867
It("should migrate the repository successfully", func() {
69-
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(mockKubeClient, username, password, upstreamURL, giteaServerRoute)
68+
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(fakeClient, username, password, upstreamURL, giteaServerRoute)
7069
Expect(err).ToNot(HaveOccurred())
7170
Expect(success).To(BeTrue())
7271
Expect(repositoryURL).To(Equal("https://gitea.example.com/user/repo"))
@@ -98,7 +97,7 @@ var _ = Describe("MigrateGiteaRepo", func() {
9897
})
9998

10099
It("should not migrate the repository and return the existing repository URL", func() {
101-
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(mockKubeClient, username, password, upstreamURL, giteaServerRoute)
100+
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(fakeClient, username, password, upstreamURL, giteaServerRoute)
102101
Expect(err).ToNot(HaveOccurred())
103102
Expect(success).To(BeTrue())
104103
Expect(repositoryURL).To(Equal("https://gitea.example.com/user/repo"))
@@ -109,7 +108,7 @@ var _ = Describe("MigrateGiteaRepo", func() {
109108
It("should return an error", func() {
110109
// Use an invalid Gitea server route to simulate client creation failure
111110
invalidRoute := "http://invalid-url"
112-
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(mockKubeClient, username, password, upstreamURL, invalidRoute)
111+
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(fakeClient, username, password, upstreamURL, invalidRoute)
113112
Expect(err).To(HaveOccurred())
114113
Expect(success).To(BeFalse())
115114
Expect(repositoryURL).To(BeEmpty())
@@ -129,7 +128,7 @@ var _ = Describe("MigrateGiteaRepo", func() {
129128
})
130129

131130
It("should return an error", func() {
132-
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(mockKubeClient, username, password, upstreamURL, giteaServerRoute)
131+
success, repositoryURL, err := giteaOperations.MigrateGiteaRepo(fakeClient, username, password, upstreamURL, giteaServerRoute)
133132
Expect(err).To(HaveOccurred())
134133
Expect(success).To(BeFalse())
135134
Expect(repositoryURL).To(BeEmpty())

‎internal/controller/kube.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,10 @@ func getRoute(routeClient routeclient.Interface, routeName, namespace string) (s
125125
}
126126

127127
// Get a Secret instance
128-
func getSecret(fullClient kubernetes.Interface, name, ns string) (*v1.Secret, error) {
129-
secret, err := fullClient.CoreV1().Secrets(ns).Get(context.Background(), name, metav1.GetOptions{})
128+
func getSecret(cl kubeclient.Client, name, ns string) (*v1.Secret, error) {
129+
secret := &v1.Secret{}
130+
err := cl.Get(context.Background(), types.NamespacedName{Name: name, Namespace: ns}, secret)
130131
if err != nil {
131-
if errors.IsNotFound(err) {
132-
return nil, err
133-
}
134132
return nil, err
135133
}
136134
return secret, nil

0 commit comments

Comments
 (0)