kapp-controller icon indicating copy to clipboard operation
kapp-controller copied to clipboard

Show target cluster in the list of apps

Open wolfedale opened this issue 2 years ago • 4 comments

What this PR does / why we need it:

This is for the issue: https://github.com/carvel-dev/kapp-controller/issues/1103

Does this PR introduce a user-facing change?

"NONE"

Additional Notes for your reviewer:

Review Checklist:
  • [ ] Follows the developer guidelines
  • [ ] Relevant tests are added or updated
  • [ ] Relevant docs in this repo added or updated
  • [ ] Relevant carvel.dev docs added or updated in a separate PR and there's a link to that PR
  • [ ] Code is at least as readable and maintainable as it was before this change

Additional documentation e.g., Proposal, usage docs, etc.:


wolfedale avatar Feb 27 '23 13:02 wolfedale

Just following up here, we had a chat about this at our community meeting this week. Generally everyone was onboard with the idea of current / this as a default rather than blank as that might indicate to the operator that something is still going on.

The current approach is to use the kubeconfig name for the external cluster. That might be a bit weird if you kubeconfig name doesn't actually mention the cluster or has other stuff in it. Ideally this is actually the name of the cluster from within the kubeconfig (kubeconfig's can also have multiple cluster info within them)

neil-hickey avatar Mar 09 '23 17:03 neil-hickey

I think another option instead of empty that makes sense is in-cluster. Regarding the name, i agree the secret name is not optimal. Best would be to get the clustername of the current context set in the kubeconfig eithin the secret. This adds logic that would be needed but would make the UX much better. Another option that could be looked at is similar to argo where the target field is actually the clusters api endpoint. So for local you would have kubernetes.default.svc.cluster.local and for others you would parse the api endpoint from the kubeconfig. Names may be more user friendly if naming is done well, but api endpoint is definitive and is the actual target.

vrabbi avatar Mar 13 '23 08:03 vrabbi

How about creating another field for ClusterTarget and check if .spec.cluster. contain data. If something is there, we can read and parse .spec.cluster.kubeconfigSecretRef., find selected cluster name and add it to the ClusterTarget field. If not, add in-cluster or this to it. Does it make sense?

wolfedale avatar Mar 14 '23 09:03 wolfedale

Any comment? :-)

wolfedale avatar Apr 18 '23 18:04 wolfedale