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 #45

Closed carlmes closed 4 days ago

carlmes commented 1 month ago

The Git repository locations are currently hard coded into the source yaml. This means that when contributing updates to this repo, either through a branch or a forked repository, the manual repo locations must be changed for testing, and then reverted prior to PR merging. Here's an example of this in a recent pull request:

  path: /spec/source/repoURL
  # TODO Change back when ready to merge back to upstream.
  # value: 'https://github.com/redhat-ai-services/ai-accelerator.git'
  value: 'https://github.com/fgharo/ai-accelerator.git'

To compound the problem, we now have multiple yaml files containing the comment "Update me on fork" with a hard coded URL, for example all the files in the cluster/overlays contain this logic with a hard coded Git repo location:

- op: replace
  path: /spec/source/repoURL
  value: 'https://github.com/redhat-ai-services/ai-accelerator.git'  # Update me on fork

Potential Enhancement #1

Use Kustomize environment variables so that there is a single location for the Git repository URL and branch name, this removes the requirement for multiple repository locations in multiple files to be updated during testing on a forked repository. It will also make it a lot easier to use this project in a disconnected environment, where GitHub is potentially unreachable and an alternate internal Git repo is used instead.

Steps to implement:

Potential Enhancement #2

To solve the problem of merge requests containing accidental "TODO Change back when ready to merge" comments, we could make these Kustomize environment variables dynamically generated during the execution of bootstrap.sh.

It's suggested that the current Git repo (git remote -v) and branch (git branch) are dynamically added to a Kustomize environment file, such as environment-properties.env. To ensure that the user is aware of the Git repo and branch in use, the bootstrap script should be enhanced with a prompt such as:

Your environment will be provisioned through ArgoCD using the following Git repo, you can use default (press Enter) or change it:
- Git Repository [https://github.com/redhat-ai-services/ai-accelerator.git]: 
- Branch [main]: 

Where the defaults in the prompt are the Git remote origin and branch from the directory where the bootstrap script is being run.

Steps to implement:

Potential Enhancement #3

Another option is to use Git actions to verify and/or update the manually defined Git repo locations in the various YAML files. This has the advantage of keeping the code looking a little cleaner (no environment to worry about), but will require a little more maintenance in the long run to keep the list of files and locations up to date.

strangiato commented 1 month ago

46 is attempting to address enhancement option 3.

The goal of this is to provide a simple reminder to the user that they should be reverting those when they open a PR.

The script uses GitHub Actions annotation format to automatically annotate the line with the issue:

Screenshot 2024-09-26 at 3 49 37 PM

I think that there is still an opportunity to enhance the bootstrap process to automatically set some of these values when users are forking/branching.

strangiato commented 1 month ago

For the env variable piece, the vars feature in kustomize that the linked article in # 2 uses is a depreciated feature that has been replaced with replacements

sukantadash commented 1 month ago

I attempted enhancement #2, but was unsuccessful. If we try to update the repo as part local env var and the same is not in the git repo then the argocd application cluster-config-app-of-apps will revert back to the git repo version when the next sync happens.

If the user wants to change the repository in their forked repo, the bootstrap process requires modifying and pushing the files from the current repository to the user-specified target repository for the changes to take effect in the ArgoCD application cluster-config-app-of-apps.

So we can try below approach. If there is a mismatch between the GitOps-configured repository and the current working repository, prompt the user with the following message:

'Your GitOps process is currently configured to use the repository <repo-url> on branch <repo-branch>, but your current working repository is <working-repo-url> on branch <working-repo-branch>. Do you want to reconfigure the GitOps process to use the current working repository? (Y/N): N'

If the user selects 'Y', update the configuration files accordingly and push them to the working repository.

Please feel free to share any suggestions or feedback on this approach.

sukantadash commented 1 month ago

Enhancement for Git Repository Locations option-2 #52

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