declarative-openshift icon indicating copy to clipboard operation
declarative-openshift copied to clipboard

GitOps for managementCluster

Open pacopeng opened this issue 10 months ago • 7 comments

This PR is for Task: https://github.com/redhat-cop/declarative-openshift/issues/48

  1. Create a folder to store management Cluster yaml files
  2. Create .bootstrap folder to store GitOps bootstrap yaml files

pacopeng avatar Apr 12 '24 04:04 pacopeng

Please help to review . Thanks.

pacopeng avatar Apr 12 '24 04:04 pacopeng

@senthilredhat adjust the bootstrap with helm charts. image

pacopeng avatar May 06 '24 04:05 pacopeng

I close this. PR , cause PR 46 already include all the commit.

pacopeng avatar May 10 '24 05:05 pacopeng

re-opening - let's review this one first

oybed avatar May 10 '24 06:05 oybed

@pacopeng I'm currently OOO, but will review this early next week.

oybed avatar May 10 '24 06:05 oybed

Overall good work to get to this point!

Below is my feedback. We can discuss when we meet next:

  • The app-of-app is an outdated approach and we should move to use ApplicationSet instead.
  • With the use of an ApplicationSet, we should also consider removing the kustomize approach as the ApplicationSet can do the same (+ more), and I'd argue in a more elegant way.
  • IMHO we should eliminate the manual steps (i.e.: option 2) in the README as that was mostly just a way to get us started. With the introduction of GitOps, we should just use that approach.
  • An ultimate goal should be for someone to just clone this repo and use it as-is for starter (e.g.: a demo/PoC, etc.) ... it may be hard to do 100% re: OCM token, etc., but we should at least default the values that can be, i.e.: this git repo, main branch, etc.

Nitpicks:

  • Please stay consistent between camelCase v.s. under_score approach

oybed avatar May 13 '24 18:05 oybed

@oybed @senthilredhat

  1. Change app of apps to applicationSet
  2. Refine README
  3. Disable rosa cluster creation for now (it require prerequisite come from crossplane,once this PR merged we can add the integration of crossplane for prerequisites)

pacopeng avatar May 16 '24 05:05 pacopeng