Skip to content

feat: Enhance ArgoCD CLI: Dynamic Repo Server Retrieval with --core and --refresh Flags #17613

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

Merged
ishitasequeira merged 19 commits into from
Mar 29, 2024
Merged

feat: Enhance ArgoCD CLI: Dynamic Repo Server Retrieval with --core and --refresh Flags #17613

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
87bc574
add const key value for ComponentRepoServer
Mangaal Mar 25, 2024
4987796
update NewRepoServerClient() to look for service with ComponentRepoS…
Mangaal Mar 25, 2024
29b0e7a
add comment for the new constants
Mangaal Mar 25, 2024
3b060b4
Merge branch 'master' of https://github.com/Mangaal/argo-cd into argo…
Mangaal Mar 26, 2024
5319368
Merge branch 'master' into argocd-cli-core-flag
Mangaal Mar 26, 2024
f67255d
Merge branch 'argocd-cli-core-flag' of https://github.com/Mangaal/arg…
Mangaal Mar 26, 2024
69713b1
Merge branch 'master' into argocd-cli-core-flag
Mangaal Mar 27, 2024
ec4b22a
instead of passing nil which leads to nil ptr referance error, pass …
Mangaal Mar 27, 2024
dde9a3e
check for operator install repo server name
Mangaal Mar 27, 2024
eb41e7c
handle empty nil ptr dereference error
Mangaal Mar 27, 2024
22e28e2
Merge branch 'argocd-cli-core-flag' of https://github.com/Mangaal/arg…
Mangaal Mar 27, 2024
2610292
Merge branch 'master' into argocd-cli-core-flag
Mangaal Mar 27, 2024
6049c22
Merge branch 'master' into argocd-cli-core-flag
Mangaal Mar 28, 2024
4c30993
Merge branch 'argocd-cli-core-flag' of https://github.com/Mangaal/arg…
Mangaal Mar 28, 2024
58a7e28
handle nil prt dereference
Mangaal Mar 28, 2024
4b1ccee
Merge branch 'master' into argocd-cli-core-flag
Mangaal Mar 28, 2024
fdba5b1
Merge branch 'argocd-cli-core-flag' of https://github.com/Mangaal/arg…
Mangaal Mar 28, 2024
d051cab
typo correction
Mangaal Mar 28, 2024
39e5371
run clidocsgen
Mangaal Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions cmd/argocd/commands/admin/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/argoproj/argo-cd/v2/controller"
"github.com/argoproj/argo-cd/v2/controller/cache"
"github.com/argoproj/argo-cd/v2/controller/metrics"
"github.com/argoproj/argo-cd/v2/controller/sharding"
argocdclient "github.com/argoproj/argo-cd/v2/pkg/apiclient"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned"
Expand Down Expand Up @@ -269,18 +270,26 @@ func NewReconcileCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command

var result []appReconcileResult
if refresh {
appClientset := appclientset.NewForConfigOrDie(cfg)
kubeClientset := kubernetes.NewForConfigOrDie(cfg)
if repoServerAddress == "" {
printLine("Repo server is not provided, trying to port-forward to argocd-repo-server pod.")
overrides := clientcmd.ConfigOverrides{}
repoServerPodLabelSelector := common.LabelKeyAppName + "=" + clientOpts.RepoServerName
repoServerName := clientOpts.RepoServerName
repoServererviceLabelSelector := common.LabelKeyComponentRepoServer + "=" + common.LabelValueComponentRepoServer

@ishitasequeira ishitasequeira Mar 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: repoServerServiceLabelSelector

@Mangaal Mangaal Mar 28, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, @ishitasequeira, Updating the same.

repoServerServices, err := kubeClientset.CoreV1().Services(namespace).List(context.Background(), v1.ListOptions{LabelSelector: repoServererviceLabelSelector})
errors.CheckError(err)
if len(repoServerServices.Items) > 0 {
if repoServerServicelabel, ok := repoServerServices.Items[0].Labels[common.LabelKeyAppName]; ok && repoServerServicelabel != "" {
repoServerName = repoServerServicelabel
}
}
repoServerPodLabelSelector := common.LabelKeyAppName + "=" + repoServerName
repoServerPort, err := kubeutil.PortForward(8081, namespace, &overrides, repoServerPodLabelSelector)
errors.CheckError(err)
repoServerAddress = fmt.Sprintf("localhost:%d", repoServerPort)
}
repoServerClient := reposerverclient.NewRepoServerClientset(repoServerAddress, 60, reposerverclient.TLSConfiguration{DisableTLS: false, StrictValidation: false})

appClientset := appclientset.NewForConfigOrDie(cfg)
kubeClientset := kubernetes.NewForConfigOrDie(cfg)
result, err = reconcileApplications(ctx, kubeClientset, appClientset, namespace, repoServerClient, selector, newLiveStateCache, serverSideDiff)
errors.CheckError(err)
} else {
Expand Down Expand Up @@ -437,5 +446,5 @@ func reconcileApplications(
}

func newLiveStateCache(argoDB db.ArgoDB, appInformer kubecache.SharedIndexInformer, settingsMgr *settings.SettingsManager, server *metrics.MetricsServer) cache.LiveStateCache {
return cache.NewLiveStateCache(argoDB, appInformer, settingsMgr, kubeutil.NewKubectl(), server, func(managedByApp map[string]bool, ref apiv1.ObjectReference) {}, nil, argo.NewResourceTracking())
return cache.NewLiveStateCache(argoDB, appInformer, settingsMgr, kubeutil.NewKubectl(), server, func(managedByApp map[string]bool, ref apiv1.ObjectReference) {}, &sharding.ClusterSharding{}, argo.NewResourceTracking())
}
18 changes: 16 additions & 2 deletions cmd/argocd/commands/headless/headless.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/redis/go-redis/v9"
log "github.com/sirupsen/logrus"
"github.com/spf13/pflag"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/kubernetes"
cache2 "k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -115,14 +116,27 @@ type forwardRepoClientset struct {
repoClientset repoapiclient.Clientset
err error
repoServerName string
kubeClientset kubernetes.Interface
}

func (c *forwardRepoClientset) NewRepoServerClient() (io.Closer, repoapiclient.RepoServerServiceClient, error) {
c.init.Do(func() {
overrides := clientcmd.ConfigOverrides{
CurrentContext: c.context,
}
repoServerPodLabelSelector := common.LabelKeyAppName + "=" + c.repoServerName
repoServerName := c.repoServerName
repoServererviceLabelSelector := common.LabelKeyComponentRepoServer + "=" + common.LabelValueComponentRepoServer
repoServerServices, err := c.kubeClientset.CoreV1().Services(c.namespace).List(context.Background(), v1.ListOptions{LabelSelector: repoServererviceLabelSelector})

@anandf anandf Mar 25, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of deriving the argocd instance name from the services, can we not apply the label app.kubernetes.io/component=repo-server to the spec.template.metadata section of the repo server deployment and then use this label to get the right repo server.
https://github.com/argoproj/argo-cd/blob/f87897c53c6f04426953f5d3ca781d3240186a60/manifests/base/repo-server/argocd-repo-server-deployment.yaml#L16C9-L16C51

@Mangaal Mangaal Mar 26, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @anandf, I will update the implementation and add the labels "app.kubernetes.io/component=repo-server" to repo-server replica pods in the manifest

@anandf anandf Mar 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Switching to app.kubernetes.io/component=repo-server component based label might cause confusion when customer provides argument --repo-server-name with the name of the argocd instance argocd-repo-server. So let's keep the logic of deriving the argocd repo server name from the service name.

if err != nil {
c.err = err
return
}
if len(repoServerServices.Items) > 0 {
if repoServerServicelabel, ok := repoServerServices.Items[0].Labels[common.LabelKeyAppName]; ok && repoServerServicelabel != "" {
repoServerName = repoServerServicelabel
}
}
repoServerPodLabelSelector := common.LabelKeyAppName + "=" + repoServerName
repoServerPort, err := kubeutil.PortForward(8081, c.namespace, &overrides, repoServerPodLabelSelector)
if err != nil {
c.err = err
Expand Down Expand Up @@ -237,7 +251,7 @@ func MaybeStartLocalServer(ctx context.Context, clientOpts *apiclient.ClientOpti
KubeClientset: kubeClientset,
Insecure: true,
ListenHost: *address,
RepoClientset: &forwardRepoClientset{namespace: namespace, context: ctxStr, repoServerName: clientOpts.RepoServerName},
RepoClientset: &forwardRepoClientset{namespace: namespace, context: ctxStr, repoServerName: clientOpts.RepoServerName, kubeClientset: kubeClientset},
EnableProxyExtension: false,
})
srv.Init(ctx)
Expand Down
4 changes: 4 additions & 0 deletions common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ const (
// AnnotationKeyAppSkipReconcile tells the Application to skip the Application controller reconcile.
// Skip reconcile when the value is "true" or any other string values that can be strconv.ParseBool() to be true.
AnnotationKeyAppSkipReconcile = "argocd.argoproj.io/skip-reconcile"
// LabelKeyComponentRepoServer is the label key to identify the component as repo-server
LabelKeyComponentRepoServer = "app.kubernetes.io/component"
// LabelValueComponentRepoServer is the label value for the repo-server component
LabelValueComponentRepoServer = "repo-server"
)

// Environment variables for tuning and debugging Argo CD
Expand Down
2 changes: 1 addition & 1 deletion controller/cache/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func (c *liveStateCache) getCluster(server string) (clustercache.ClusterCache, e
return nil, fmt.Errorf("error getting cluster: %w", err)
}

if !c.canHandleCluster(cluster) {
if !c.canHandleCluster(cluster) && c.clusterSharding == nil {

@anandf anandf Mar 27, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the nil check should come first isn't it ?

@Mangaal Mangaal Mar 28, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @anandf, I have updated the code to handle nil ptr dereference separately

return nil, fmt.Errorf("controller is configured to ignore cluster %s", cluster.Server)
}

Expand Down