redhat-ai-services / ai-accelerator

The AI Accelerator is a template project for setting up Red Hat OpenShift AI using GitOps
28 stars 59 forks source link

Enhancement for Git Repository Locations option-2 #52

Closed sukantadash closed 1 month ago

sukantadash commented 1 month ago

Please review the changes.

  1. User will be given option to change the repository to the forked/local repo while running bootstrap script.
  2. This change of repository is temporary change and doesn't commit to the git repo hence the ArgoCD application cluster-config-app-of-apps remains as out-of-sync in ArgoCD. Donot sync this application. if synced then the repository will be replaced with the redhat-ai-service/ai-accelerator.git
  3. Everytime when the bootstrap script runs, it will be given option to change the repo to the forked/local repo
  4. Now the repository change in one place that is in the file ./components/argocd/apps/base/cluster-config-app-of-apps.yaml
  5. If user wants to use a different repository with respect respect to different overlay then they have to uncomment the patching code in the respective kustomization.yaml file

e.g. /clusters/overlays/rhoai-stable-2.10/kustomization.yaml

#Uncomment below patching for repo setup for each overlay
#patches:
# set the repo and branch for applications
#- path: patch-application-repo-revision.yaml
#  target:
#    group: argoproj.io
#    kind: Application
sukantadash commented 1 month ago

Updated "Installation.md"

By default, the script uses the repository (https://github.com/redhat-ai-services/ai-accelerator.git) and the main branch. The script will prompt for a different repository and branch if desired. If alternate values are provided, the following will occur:

strangiato commented 1 month ago

There are some ideas I think I like here, and a few I don't.

I was originally hesitant to like the idea of not having the patch already configured on all of the cluster folders but I feel like I am starting to come around to the idea. The fact that we can add the patch in when needed feels like a good compromise when we want to make a change for a specific cluster, and having the config only in the app of apps does make the setting feel a bit simpler.

I'm not crazy about the fact that the changes are not getting committed back to the repo and that we are simply relying "not syncing" for the changes.

I think we would be better off committing the changes, and then adding in the checks on #46 (with some updates to account for the change in location) to verify the repo was reset on a PR back to our repo.

Instead of hardcoding the repo URL into the bootstrap script we could possibly try to incorporate some of the ideas I was playing around with in #54 where I am detecting the repo URL from git config --get remote.origin.url and using that to compare to what is set in the repo to check if it needs to be updated.

Also, there are a few checks that exist in the bootstrap script already around the branch that will need to be updated to work with your changes here.

@sukantadash what would you think about merging that to a branch like repo-bootstrap-updates and I can try and incorporate some of your changes with some of the things I was trying?

sukantadash commented 1 month ago

@strangiato looks like I don't have access to create branch repo-bootstrap-updates. I like the idea of committing back the changes to the repo and I have some code changes related to that in my local I will push them to the PR.

sukantadash commented 1 month ago

@strangiato I have created a new branch repo-bootstrap-updates and merged the changes to that branch.