Skip to content

Commit

Permalink
ACM-4489 Remove cached client & disable secret caching
Browse files Browse the repository at this point in the history 
Signed-off-by: Jonathan Marcantonio <jmarcant@redhat.com>

Grab only necessary secrets

Signed-off-by: Jonathan Marcantonio <jmarcant@redhat.com>

Lint files

Signed-off-by: Jonathan Marcantonio <jmarcant@redhat.com>

Disable cache for secrets

Signed-off-by: Jonathan Marcantonio <jmarcant@redhat.com>

Replace client with Non-caching client

Signed-off-by: Jonathan Marcantonio <jmarcant@redhat.com>

Replace client with non-caching client

Signed-off-by: Jonathan Marcantonio <jmarcant@redhat.com>
  • Loading branch information
@lennysgarage
lennysgarage committed Apr 17, 2023
1 parent e58a694 commit a1b0d1b
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 128 deletions.
7 changes: 7 additions & 0 deletions cmd/appsubsummary/exec/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"open-cluster-management.io/multicloud-operators-subscription/pkg/controller"
"open-cluster-management.io/multicloud-operators-subscription/pkg/utils"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
k8swebhook "sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand Down Expand Up @@ -78,6 +80,7 @@ func RunManager() {
RenewDeadline: &renewDeadline,
RetryPeriod: &retryPeriod,
WebhookServer: &k8swebhook.Server{TLSMinVersion: "1.2"},
NewClient: NewNonCachingClient,
})
if err != nil {
klog.Error(err, "")
Expand Down Expand Up @@ -114,3 +117,7 @@ func RunManager() {
os.Exit(1)
}
}

func NewNonCachingClient(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
return client.New(config, client.Options{})
}
7 changes: 7 additions & 0 deletions cmd/manager/exec/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"k8s.io/klog/v2"
addonV1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
Expand Down Expand Up @@ -122,6 +124,7 @@ func RunManager() {
RenewDeadline: &renewDeadline,
RetryPeriod: &retryPeriod,
WebhookServer: &k8swebhook.Server{TLSMinVersion: "1.2"},
NewClient: NewNonCachingClient,
})

if err != nil {
Expand Down Expand Up @@ -362,3 +365,7 @@ func serveHealthProbes(healthProbeBindAddress string, configCheck healthz.Checke

return server.ListenAndServe()
}

func NewNonCachingClient(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
return client.New(config, client.Options{})
}
24 changes: 18 additions & 6 deletions pkg/controller/spoketoken/spoke_toke_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ var (
},
Secrets: []corev1.ObjectReference{
{
Name: "application-manager-dockercfg-tlxd5",
Name: "application-manager-dockercfg-1",
},
},
}
Expand All @@ -74,7 +74,19 @@ var (
},
Secrets: []corev1.ObjectReference{
{
Name: "application-manager-dockercfg-tlxd5",
Name: "application-manager-dockercfg-1",
},
},
}

dockerSecret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "application-manager-dockercfg-1",
Namespace: "open-cluster-management-agent-addon",
Annotations: map[string]string{
"kubernetes.io/service-account.name": "application-manager",
"openshift.io/token-secret.name": "application-manager-token-1",
"openshift.io/token-secret.value": "dummy-1",
},
},
}
Expand All @@ -85,9 +97,6 @@ var (
Namespace: "open-cluster-management-agent-addon",
Annotations: map[string]string{"kubernetes.io/service-account.name": "application-manager"},
},
Data: map[string][]byte{
"token": []byte("ZHVtbXkxCg=="),
},
Type: corev1.SecretTypeServiceAccountToken,
}
)
Expand Down Expand Up @@ -133,6 +142,9 @@ func TestReconcile(t *testing.T) {
// Create the addon agent service account.
g.Expect(c.Create(context.TODO(), sa1)).NotTo(gomega.HaveOccurred())

g.Expect(c.Create(context.TODO(), dockerSecret)).NotTo(gomega.HaveOccurred())
defer c.Delete(context.TODO(), dockerSecret)

// Create the addon agent service account token secret and reconcile.
g.Expect(c.Create(context.TODO(), secret1)).NotTo(gomega.HaveOccurred())
defer c.Delete(context.TODO(), secret1)
Expand All @@ -153,7 +165,7 @@ func TestReconcile(t *testing.T) {

configData := &Config{}
g.Expect(json.Unmarshal(theSecret.Data["config"], configData)).NotTo(gomega.HaveOccurred())
g.Expect(configData.BearerToken).To(gomega.Equal(string(secret1.Data["token"])))
g.Expect(configData.BearerToken).To(gomega.Equal(dockerSecret.Annotations["openshift.io/token-secret.value"]))
g.Expect(configData.TLSClientConfig["insecure"]).To(gomega.BeTrue())

// Verify the labels
Expand Down
50 changes: 27 additions & 23 deletions pkg/controller/spoketoken/spoke_token_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,15 @@ func (r *ReconcileAgentToken) Reconcile(ctx context.Context, request reconcile.R
}

// Get the service account token from the service account's secret list
saSecret := r.getServiceAccountTokenSecret()
token := r.getServiceAccountTokenSecret()

if saSecret == nil {
if token == "" {
klog.Error("Failed to find the service account token.")
return reconcile.Result{}, errors.New("failed to find the klusterlet agent addon service account token secret")
}

token, ok := saSecret.Data["token"]

if !ok {
klog.Error("Serviceaccount token secret does not contain token.")
return reconcile.Result{}, errors.New("the klusterlet agent addon service account token secret does not contain token")
}

// Prepare the secret to be created/updated in the managed cluster namespace on the hub
secret := r.prepareAgentTokenSecret(string(token))
secret := r.prepareAgentTokenSecret(token)

// Get the existing secret in the managed cluster namespace from the hub
hubSecret := &corev1.Secret{}
Expand Down Expand Up @@ -248,27 +241,38 @@ func (r *ReconcileAgentToken) prepareAgentTokenSecret(token string) *corev1.Secr
return mcSecret
}

func (r *ReconcileAgentToken) getServiceAccountTokenSecret() *corev1.Secret {
// Get all secrets
// list thing for rolling update check
secretList := &corev1.SecretList{}
listopts := &client.ListOptions{Namespace: "open-cluster-management-agent-addon"}
err := r.Client.List(context.TODO(), secretList, listopts)
func (r *ReconcileAgentToken) getServiceAccountTokenSecret() string {
// Grab application-manager service account
sa := &corev1.ServiceAccount{}

err := r.Client.Get(context.TODO(), types.NamespacedName{Name: "application-manager", Namespace: "open-cluster-management-agent-addon"}, sa)
if err != nil {
klog.Error(err.Error())
return nil
return ""
}

for _, secret := range secretList.Items {
// Get the application-manager service account token
if secret.Type == corev1.SecretTypeServiceAccountToken && strings.HasPrefix(secret.Name, "application-manager-token-") {
klog.Info("found the application-manager service account token secret " + secret.Name)
return &secret
// loop through secrets to find application-manager-dockercfg secret
for _, secret := range sa.Secrets {
if strings.HasPrefix(secret.Name, "application-manager-dockercfg") {
klog.Info("found the application-manager-dockercfg secret " + secret.Name)

// application-manager-token secret is owned by the dockercfg secret
dockerSecret := &corev1.Secret{}

err = r.Client.Get(context.TODO(), types.NamespacedName{Name: secret.Name, Namespace: "open-cluster-management-agent-addon"}, dockerSecret)
if err != nil {
klog.Error(err.Error())
return ""
}

anno := dockerSecret.GetAnnotations()
klog.Info("found the application-manager-token secret " + anno["openshift.io/token-secret.name"])

return anno["openshift.io/token-secret.value"]
}
}

return nil
return ""
}

// getKubeAPIServerAddress - Get the API server address from OpenShift kubernetes cluster. This does not work with other kubernetes.
Expand Down
45 changes: 0 additions & 45 deletions pkg/synchronizer/kubernetes/cached_client.go
View file

This file was deleted.

6 changes: 0 additions & 6 deletions pkg/synchronizer/kubernetes/sync_appsubstatus_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,6 @@ var _ = Describe("test create/update/delete appsub status for standalone and man

s := &KubeSynchronizer{
Interval: 0,
localCachedClient: &cachedClient{},
remoteCachedClient: &cachedClient{},
LocalClient: k8sClient,
RemoteClient: k8sClient,
hub: true,
Expand Down Expand Up @@ -291,8 +289,6 @@ var _ = Describe("test create/update/delete appsub status for standalone and man

s = &KubeSynchronizer{
Interval: 0,
localCachedClient: &cachedClient{},
remoteCachedClient: &cachedClient{},
LocalClient: k8sClient,
RemoteClient: k8sClient,
hub: false,
Expand Down Expand Up @@ -407,8 +403,6 @@ var _ = Describe("test create/update/delete appsub status for standalone and man

s = &KubeSynchronizer{
Interval: 0,
localCachedClient: &cachedClient{},
remoteCachedClient: &cachedClient{},
LocalClient: k8sClient,
RemoteClient: k8sClient,
hub: false,
Expand Down
56 changes: 8 additions & 48 deletions pkg/synchronizer/kubernetes/sync_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package kubernetes

import (
"context"
"fmt"
"sync"
"time"

Expand Down Expand Up @@ -62,8 +61,6 @@ type SubscriptionClusterStatus struct {
// KubeSynchronizer handles resources to a kube endpoint.
type KubeSynchronizer struct {
Interval int
localCachedClient *cachedClient
remoteCachedClient *cachedClient
LocalClient client.Client
LocalNonCachedClient client.Client
RemoteClient client.Client
Expand Down Expand Up @@ -136,41 +133,28 @@ func CreateSynchronizer(config, remoteConfig *rest.Config, scheme *runtime.Schem
dmtx: sync.Mutex{},
}

s.localCachedClient, err = newCachedClient(config, &types.NamespacedName{Name: "local"})
if err != nil {
klog.Error("Failed to initialize client to update local status. err: ", err)

return nil, err
}

s.LocalClient = s.localCachedClient.clt

// set up non chanced local client
// set up non cached local client, the local client is the client for managed cluster
s.LocalNonCachedClient, err = client.New(config, client.Options{})
if err != nil {
klog.Error("Failed to generate client to local cluster with error: ", err)

return nil, err
}

s.LocalClient = s.LocalNonCachedClient

// the remote client is the one for the hub cluster.
s.RemoteClient = s.LocalClient
if remoteConfig != nil {
s.remoteCachedClient, err = newCachedClient(remoteConfig, syncid)
if err != nil {
klog.Error("Failed to initialize client to update remote status. err: ", err)

return nil, err
}

s.RemoteClient = s.remoteCachedClient.clt

// set up non chanced hub client
// set up non cached hub client
s.RemoteNonCachedClient, err = client.New(remoteConfig, client.Options{})
if err != nil {
klog.Error("Failed to generate client to hub cluster with error: ", err)

return nil, err
}

s.RemoteClient = s.RemoteNonCachedClient
}

defaultExtension.localClient = s.LocalClient
Expand All @@ -191,35 +175,11 @@ func CreateSynchronizer(config, remoteConfig *rest.Config, scheme *runtime.Schem
return s, nil
}

// Start caches, this will be triggered by the manager.
// this will be triggered by the manager.
func (sync *KubeSynchronizer) Start(ctx context.Context) error {
klog.Info("start synchronizer")
defer klog.Info("stop synchronizer")

go func() {
if err := sync.localCachedClient.clientCache.Start(ctx); err != nil {
klog.Error(err, "failed to start up cache")
}
}()

go func() {
if err := sync.remoteCachedClient.clientCache.Start(ctx); err != nil {
klog.Error(err, "failed to start up cache")
}
}()

if !sync.localCachedClient.clientCache.WaitForCacheSync(ctx) {
return fmt.Errorf("failed to start up local cache")
}

klog.Info("local config cache started")

if !sync.remoteCachedClient.clientCache.WaitForCacheSync(ctx) {
return fmt.Errorf("failed to start up remote cache")
}

klog.Info("remote config cache started")

return nil
}

Expand Down

0 comments on commit a1b0d1b

Please sign in to comment.