add route/ingress URL to .status #514

reginapizza · 2021-12-13T01:00:44Z

What type of PR is this?
/kind enhancement

What does this PR do / why we need it:
This would add a new field, .host, to .status of ArgoCD. When route or ingress is enabled (priority given to route) then the route will be displayed in the new URL field.

When no URL exists from a route or ingress, the field will not be displayed.

When on a non-OpenShift cluster (meaning the Route API is not available), if the user chooses to enable Route, they will only get a log saying that Routes are not available in non-OpenShift environments and to please use Ingresses instead. The state of the application controller and of the URL will not be affected.

When the route or ingress is configured, but the corresponding controller has not yet set it up properly (i.e. is not in Ready state or does not propagate its URL), this is indicated in the Operand as well in the value of .status.url as Pending instead of the URL. Also, if .status.url is Pending, this affects the overall status for the Operand by making it Pending instead of Available.

Have you updated the necessary documentation?

Documentation update is required by this PR.
Documentation has been updated.

Which issue(s) this PR fixes:
Fixes #246

How to test changes / Special notes to the reviewer:

Special Notes to the reviewer:

Whatever your cluster is, make sure that Ingress Controller is installed, enabled and running before moving forward. Instructions for ingress-controller for the following types of cluster are:

kind : https://abhishekveeramalla-av.medium.com/run-argo-cd-using-operator-on-kind-e59f48687d38
minikube: run command minikube addons enable ingress
k3d: Traefik Ingress Controller is installed/enabled by default
OpenShift: after enabling ingress in Argo CD spec, update the following on the ingress spec:
- remove Nginx annotations
- update hostame to example-argocd.yourcluster.example.com where example-argocd is the argocd host

Ingress Testing:

Get cluster and log in (I used a K3D cluster for this).
In your cloned repo on this branch, go to Makefile and change the version # (just to something else) and change image base tag to IMAGE_TAG_BASE ?= quay.io/{your quay or docker username}/argocd-operator. Then run make docker-build, make docker-pushand login to a cluster and then run make deploy
Add the Argo CD instance:

cat <<EOF | kubectl apply -f -                                                                    
apiVersion: argoproj.io/v1alpha1
kind: ArgoCD
metadata:
  name: argocd
spec: {}
EOF

Create an Ingress:

cat <<EOF | oc apply -f -                                                                    
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: argocd-server
spec:
  rules:
  - host: argocd
    http:
      paths:
      - backend:
          service:
            name: argocd-server
            port:
              number: 8080
        path: /
        pathType: ImplementationSpecific
EOF

If you do oc get ingress argocd-server you should see:

NAME            CLASS    HOSTS    ADDRESS      PORTS   AGE
argocd-server   <none>   argocd   172.22.0.3   80      9m56s

Edit the ArgoCD instance so that ingress and route (or at least just route if you're on an OpenShift cluster) is enabled with oc edit argocd argocd. spec.server should look something like:

  server:
    autoscale:
      enabled: false
    grpc:
      ingress:
        enabled: false
    ingress:
      enabled: true
    route:
      enabled: false
    service:
      type: ""
  tls:
    ca: {}

Save your changes.

Wait a second for it to update, and then check back on your CR oc get argocd argocd -o yaml. You should see that the URL has been added to the .status field.

status:
  applicationController: Running
  dex: Running
  host: 172.22.0.3
  phase: Available
  redis: Running
  repo: Running
  server: Running
  ssoConfig: Unknown

Route Testing:

Get cluster and log in (I used an OpenShift cluster for this since routes are specific to OpenShift, if the Route API is not available this functionality will not work).
In your cloned repo on this branch, go to Makefile and change the version # (just to something else) and change image base tag to IMAGE_TAG_BASE ?= quay.io/{your quay or docker username}/argocd-operator. Then run make docker-build, make docker-pushand login to a cluster and then run make deploy
Add the Argo CD instance:

cat <<EOF | kubectl apply -f -                                                                    
apiVersion: argoproj.io/v1alpha1
kind: ArgoCD
metadata:
  name: argocd
spec: {}
EOF

Edit the ArgoCD instance so that ingress and route (or at least just route if you're on an OpenShift cluster) is enabled with oc edit argocd argocd. spec.server should look something like:

  server:
    autoscale:
      enabled: false
    grpc:
      ingress:
        enabled: false
    ingress:
      enabled: false
    route:
      enabled: true
    service:
      type: ""
  tls:
    ca: {}

Save your changes.
5. Wait a second for it to update, and then check back on your CR oc get argocd argocd -o yaml. You should see that the URL has been added to the .status field.

status:
  applicationController: Running
  dex: Running
  host: argocd-server-default.apps.app-svc-4.8-120914.devcluster.openshift.com
  phase: Available
  redis: Running
  repo: Running
  server: Running
  ssoConfig: Unknown

Note: If route and ingress are both enabled, the route (if available) will have preference over ingress.

controllers/argocd/deployment.go

controllers/argocd/status.go

deploy/olm-catalog/argocd-operator/0.2.0/argoproj.io_argocds.yaml

iam-veeramalla · 2021-12-16T13:56:18Z

@reginapizza please run make bundle and resubmit the file 0.2.0/argoproj.io_argocds.yaml, my bad that I asked you to uncommit that file.

jaideepr97 · 2021-12-17T14:27:29Z

would it be a good idea to mention that the created route/ingress URL can be retrieved from the ArgoCD instance's .status.URL field in https://argocd-operator.readthedocs.io/en/latest/usage/ingress/ and https://argocd-operator.readthedocs.io/en/latest/usage/routes/?

@reginapizza

jaideepr97 · 2021-12-17T14:42:48Z

On further thought, a couple of cases occurred to me that I'm wondering how/if we should handle:

If someone tries to set .spec.server.route: enabled in a k8s cluster, this might throw an error. Maybe we should add a check for the route API here and fail silently instead?
Do we want to allow users to be able to set both route and ingress to enabled? If Both route and ingress are enabled on Openshift, is there a possibility we could end up with 2 routes for the same server? (One created by the operator, the other created by the openshift router automatically from the ingress) In that case should we just allow the route created by the router?
Let me know what you think @reginapizza @iam-veeramalla

reginapizza · 2021-12-17T19:45:10Z

@jaideepr97 I've updated my PR to address your first concern, but I'm still not sure about the second. @iam-veeramalla do you think that it's a concern?

controllers/argocd/status.go

jaideepr97 · 2021-12-22T22:50:17Z

LGTM
Thanks @reginapizza !!

api/v1alpha1/argocd_types.go

controllers/argocd/status.go

wtam2018 · 2021-12-23T00:15:07Z

controllers/argocd/status.go

+				cr.Status.Phase = status
+				return r.Client.Status().Update(context.TODO(), cr)
+			} else {
+				cr.Status.URL = route.Spec.Host


We should get the host from route.Status.Ingress which is a list of RouteIngresses. I think we should show all the ingresses.

Also, each ingress has a status. If any of the ingresses is in failed status, should cr.Status.Phase be set to failed?

controllers/argocd/status.go

api/v1alpha1/argocd_types.go

wtam2018 · 2022-02-01T20:04:53Z

Ingress and route can be enabled at the same time. However, the status does not show ingress if route is enabled. WDYT @jaideepr97 @iam-veeramalla @reginapizza Should status be honest about showing ingress as well? We can probably just document as such.

jaideepr97 · 2022-02-03T13:42:54Z

Per my understanding, on openshift environments routes are given more importance than ingresses (since a route is created automatically for each ingress) so I would say it makes sense to just show the route and document that the ingress is not shown in this case

On a related note, if both route and ingress are enabled we might end up with 2 routes for the same server deployment
Is this a case we want to handle?

wtam2018 · 2022-02-03T14:28:31Z

@jaideepr97 It is also my understanding. https://docs.openshift.com/container-platform/4.6/networking/routes/route-configuration.html#nw-ingress-creating-a-route-via-an-ingress_route-configuration You can annotate an Ingress and OpenShift can create a Route automatically,
The question came up for the case where both ingress and route are legitimately enabled by the user in the Argo CD CR. We are only showing the status of Route. The ingress does not have the annotation.
I think we just need to document that only the route status is displaced when both ingress and route are enabled.

jannfis

LGTM

wtam2018 · 2022-02-03T15:33:46Z

controllers/argocd/status.go

+			log.Info("argocd-server ingress requested but not found on cluster")
+			return nil
+		} else {
+			if !reflect.DeepEqual(ingress.Status.LoadBalancer, corev1.LoadBalancerStatus{}) && len(ingress.Status.LoadBalancer.Ingress) > 0 {


@reginapizza @jannfis I can''t figure out why is the DeepEqual() is needed.

would if !reflect.ValueOf(ingress.Status.LoadBalancer).IsZero() && len(ingress.Status.LoadBalancer.Ingress) > 0 { be better?

Yes, IsZero a bit cleaner and faster then DeepCopy and && len(ingress.Status.LoadBalancer.Ingress) > 0 can be eliminated. Leaving it as-is is fine.

I think we should keep the len(ingress.Status.LoadBalancer.Ingress) > 0 as a sanity check, to make sure code only executes if we have at least 1 proper ingress hostname/ip to work with

wtam2018 · 2022-02-03T15:36:52Z

@reginapizza please update how to test o include testing the ingress only case.

reginapizza · 2022-02-03T19:53:20Z

@wtam2018 I've updated the instructions on how to test ingresses with it!

* Improve docs for custom roles feature (#548) * add route/ingress host to .status (#514) * fix: argocd-tls-certs-cm is overwritten on any change (#553) Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com> * add docs for host field in .status (#554) * GITOPS-1550 removed unnecessary roles/rolebindings for target namespace (#557) * chore(deps): bump github.com/argoproj/argo-cd/v2 from 2.2.2 to 2.2.4 (#560) Bumps [github.com/argoproj/argo-cd/v2](https://github.com/argoproj/argo-cd) from 2.2.2 to 2.2.4. - [Release notes](https://github.com/argoproj/argo-cd/releases) - [Changelog](https://github.com/argoproj/argo-cd/blob/master/CHANGELOG.md) - [Commits](argoproj/argo-cd@v2.2.2...v2.2.4) --- updated-dependencies: - dependency-name: github.com/argoproj/argo-cd/v2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: update operator capabilities level in CSV (#549) * feat: bump argo cd to version 2.2.5 * feat: update bundle and manifests to v0.3.0 * Update deploy/olm-catalog/argocd-operator/0.2.0/argocd-operator.v0.2.0.clusterserviceversion.yaml Co-authored-by: jannfis <jann@mistrust.net> * Update config/manifests/bases/argocd-operator.clusterserviceversion.yaml Co-authored-by: jannfis <jann@mistrust.net> * Update bundle/manifests/argocd-operator.clusterserviceversion.yaml Co-authored-by: jannfis <jann@mistrust.net> Co-authored-by: Chetan Banavikalmutt <chetanrns1997@gmail.com> Co-authored-by: Regina Scott <50851526+reginapizza@users.noreply.github.com> Co-authored-by: Yi Cai <yicai@redhat.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jaideep Raghunath Rao <jaideep.r97@gmail.com> Co-authored-by: William Tam <wtam@redhat.com> Co-authored-by: jannfis <jann@mistrust.net>

iam-veeramalla · 2022-04-20T06:58:32Z

Known Issue:
@ishitasequeira and I have noticed that this feature does not work as expected when Argo CD server is exposed using an Ingress resource(instead of route) on an OpenShift cluster. Please refer to the issue or JIRA for more details.

iam-veeramalla reviewed Dec 13, 2021

View reviewed changes

reginapizza requested a review from iam-veeramalla December 15, 2021 15:28

jaideepr97 reviewed Dec 18, 2021

View reviewed changes

controllers/argocd/status.go Outdated Show resolved Hide resolved

wtam2018 reviewed Dec 22, 2021

View reviewed changes

api/v1alpha1/argocd_types.go Outdated Show resolved Hide resolved

wtam2018 reviewed Dec 22, 2021

View reviewed changes

controllers/argocd/status.go Outdated Show resolved Hide resolved

wtam2018 reviewed Dec 22, 2021

View reviewed changes

controllers/argocd/status.go Outdated Show resolved Hide resolved

wtam2018 reviewed Dec 23, 2021

View reviewed changes

controllers/argocd/status.go Outdated Show resolved Hide resolved

reginapizza changed the title ~~add route/ingress URL to .status~~ WIP: add route/ingress URL to .status Dec 23, 2021

wtam2018 reviewed Jan 25, 2022

View reviewed changes

api/v1alpha1/argocd_types.go Outdated Show resolved Hide resolved

add route/ingress host to .status

2dd198b

reginapizza changed the title ~~WIP: add route/ingress URL to .status~~ add route/ingress URL to .status Jan 27, 2022

jannfis approved these changes Feb 3, 2022

View reviewed changes

jannfis merged commit 808d0a2 into argoproj-labs:master Feb 3, 2022

wtam2018 reviewed Feb 3, 2022

View reviewed changes

reginapizza mentioned this pull request Feb 7, 2022

add docs for host field in .status #554

Merged

iam-veeramalla mentioned this pull request Feb 24, 2022

Update bundle to v0.3.0 and cherry-pick commits from master #574

Merged

2 tasks

iam-veeramalla mentioned this pull request Apr 20, 2022

Argo CD .status.host field is not updated when using Ingress on OpenShift clusters. #620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add route/ingress URL to .status #514

add route/ingress URL to .status #514

reginapizza commented Dec 13, 2021 •

edited

Loading

iam-veeramalla commented Dec 16, 2021

jaideepr97 commented Dec 17, 2021

jaideepr97 commented Dec 17, 2021

reginapizza commented Dec 17, 2021

jaideepr97 commented Dec 22, 2021

wtam2018 Dec 23, 2021 •

edited

Loading

wtam2018 commented Feb 1, 2022 •

edited

Loading

jaideepr97 commented Feb 3, 2022

wtam2018 commented Feb 3, 2022 •

edited

Loading

jannfis left a comment

wtam2018 Feb 3, 2022

reginapizza Feb 3, 2022

wtam2018 Feb 3, 2022 •

edited

Loading

jaideepr97 Feb 4, 2022

wtam2018 commented Feb 3, 2022

reginapizza commented Feb 3, 2022

iam-veeramalla commented Apr 20, 2022

add route/ingress URL to .status #514

add route/ingress URL to .status #514

Conversation

reginapizza commented Dec 13, 2021 • edited Loading

iam-veeramalla commented Dec 16, 2021

jaideepr97 commented Dec 17, 2021

jaideepr97 commented Dec 17, 2021

reginapizza commented Dec 17, 2021

jaideepr97 commented Dec 22, 2021

wtam2018 Dec 23, 2021 • edited Loading

Choose a reason for hiding this comment

wtam2018 commented Feb 1, 2022 • edited Loading

jaideepr97 commented Feb 3, 2022

wtam2018 commented Feb 3, 2022 • edited Loading

jannfis left a comment

Choose a reason for hiding this comment

wtam2018 Feb 3, 2022

Choose a reason for hiding this comment

reginapizza Feb 3, 2022

Choose a reason for hiding this comment

wtam2018 Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

jaideepr97 Feb 4, 2022

Choose a reason for hiding this comment

wtam2018 commented Feb 3, 2022

reginapizza commented Feb 3, 2022

iam-veeramalla commented Apr 20, 2022

reginapizza commented Dec 13, 2021 •

edited

Loading

wtam2018 Dec 23, 2021 •

edited

Loading

wtam2018 commented Feb 1, 2022 •

edited

Loading

wtam2018 commented Feb 3, 2022 •

edited

Loading

wtam2018 Feb 3, 2022 •

edited

Loading