search

opendatahub-io / ai-edge

ODH integration with AI at the Edge usecases
Apache License 2.0
8 stars 17 forks source link

RHOAIENG-3128: Make GitOps update pipeline more robust and flexible #236

Closed grdryn closed 4 months ago

grdryn commented 5 months ago

JIRA: https://issues.redhat.com/browse/RHOAIENG-3128

Description

The intention of this change is to make the GitOps update pipeline both more robust and more flexible. Before this change, this pipeline was very tied to the way we do things in the example GitOps repositories that we've got in this repo, and users didn't have the ability to deviate from that very much.

This proposed change allows them to specify a script to be run, with the intention that they'll specify one or more yq commands, to update whichever files in their GitOps repository need to be updated with the new image details. IMAGE_NAME and IMAGE_DIGEST for the new image will be available as environment variables to the script at runtime. See the examples for example commands/scripts.

This PR also fixes a minor bug where the git-clone task would just checkout the default branch (i.e. main) despite what the user had specified in the PipelineRun using the gitRepoBaseBranch parameter. While this was previously causing the PR to be correctly created against the expected target branch, the change wasn't being made on top of that branch. This would cause issues if the file(s) that the GitOps Update pipeline was targeting didn't exist on the default branch (would cause pipeline error), or were different enough from the version of the file(s) on the target branch (would cause an unmergeable PR).

Breaking change: The imageReferenceFilePath parameter has been removed, and the yq-script has been added. In a previous iteration of this PR, the yq-script had a default value, but this has been removed now, since it referred to the imageReferenceFilePath param. Instead, each of the example PipelineRuns contain a yq-script value equivalent to the previous behaviour.

How Has This Been Tested?

~This has just been tested with the existing examples in this repo so far. I will update here when I've tried it with different scenarios, per the acceptance criteria in the Jira issue.~ This has been tested with the pre-existing examples in the repo here (see example PR for bike-rental-app, and example PR for tensorflow-housing-app), and also with the new JSON example GitOps repo file that this PR adds (see example PR).

Merge criteria:

openshift-ci-robot commented 5 months ago

@grdryn: This pull request references RHOAIENG-3128 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to [this](https://github.com/opendatahub-io/ai-edge/pull/236): > >JIRA: https://issues.redhat.com/browse/RHOAIENG-3128 > >## Description > >The intention of this change is to make the GitOps update pipeline both more robust and more flexible. Before this change, this pipeline was very tied to the way we do things in the example GitOps repositories that we've got in this repo, and users didn't have the ability to deviate from that very much. > >## How Has This Been Tested? > > > >This has just been tested with the existing examples in this repo so far. I will update here when I've tried it with different scenarios, per the acceptance criteria in the Jira issue. > >## Merge criteria: > > > >- [ ] The commits are squashed in a cohesive manner and have meaningful messages. >- [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). >- [ ] The developer has manually tested the changes and verified that the changes work > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=opendatahub-io%2Fai-edge). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
grdryn commented 4 months ago

/remove-approve

(new process)

openshift-ci[bot] commented 4 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MarianMacik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/opendatahub-io/ai-edge/blob/main/OWNERS)~~ [MarianMacik] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-ci-robot commented 4 months ago

@grdryn: This pull request references RHOAIENG-3128 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to [this](https://github.com/opendatahub-io/ai-edge/pull/236): > >JIRA: https://issues.redhat.com/browse/RHOAIENG-3128 > >## Description > >The intention of this change is to make the GitOps update pipeline both more robust and more flexible. Before this change, this pipeline was very tied to the way we do things in the example GitOps repositories that we've got in this repo, and users didn't have the ability to deviate from that very much. > >This proposed change allows them to specify a script to be run, with the intention that they'll specify one or more `yq` commands, to update whichever files in their GitOps repository need to be updated with the new image details. `IMAGE_NAME` and `IMAGE_DIGEST` for the new image will be available as environment variables to the script at runtime. See the examples for example commands/scripts. > >This PR also fixes a minor bug where the `git-clone` task would just checkout the default branch (i.e. `main`) despite what the user had specified in the PipelineRun using the `gitRepoBaseBranch` parameter. While this was previously causing the PR to be correctly created against the expected target branch, the change wasn't being made on top of that branch. This would cause issues if the file(s) that the GitOps Update pipeline was targeting didn't exist on the default branch (would cause pipeline error), or were different enough from the version of the file(s) on the target branch (would cause an unmergeable PR). > >## How Has This Been Tested? > > > >~This has just been tested with the existing examples in this repo so far. I will update here when I've tried it with different scenarios, per the acceptance criteria in the Jira issue.~ >This has been tested with the pre-existing examples in the repo here (see [example PR](https://github.com/grdryn/ai-edge/pull/12)), and also with the new JSON example GitOps repo file that this PR adds (see [example PR](https://github.com/grdryn/ai-edge/pull/12)). > >## Merge criteria: > > > >- [x] The commits are squashed in a cohesive manner and have meaningful messages. >- [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). >- [x] The developer has manually tested the changes and verified that the changes work > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=opendatahub-io%2Fai-edge). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 4 months ago

@grdryn: This pull request references RHOAIENG-3128 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to [this](https://github.com/opendatahub-io/ai-edge/pull/236): > >JIRA: https://issues.redhat.com/browse/RHOAIENG-3128 > >## Description > >The intention of this change is to make the GitOps update pipeline both more robust and more flexible. Before this change, this pipeline was very tied to the way we do things in the example GitOps repositories that we've got in this repo, and users didn't have the ability to deviate from that very much. > >This proposed change allows them to specify a script to be run, with the intention that they'll specify one or more `yq` commands, to update whichever files in their GitOps repository need to be updated with the new image details. `IMAGE_NAME` and `IMAGE_DIGEST` for the new image will be available as environment variables to the script at runtime. See the examples for example commands/scripts. > >This PR also fixes a minor bug where the `git-clone` task would just checkout the default branch (i.e. `main`) despite what the user had specified in the PipelineRun using the `gitRepoBaseBranch` parameter. While this was previously causing the PR to be correctly created against the expected target branch, the change wasn't being made on top of that branch. This would cause issues if the file(s) that the GitOps Update pipeline was targeting didn't exist on the default branch (would cause pipeline error), or were different enough from the version of the file(s) on the target branch (would cause an unmergeable PR). > >## How Has This Been Tested? > > > >~This has just been tested with the existing examples in this repo so far. I will update here when I've tried it with different scenarios, per the acceptance criteria in the Jira issue.~ >This has been tested with the pre-existing examples in the repo here (see [example PR for bike-rental-app](https://github.com/grdryn/ai-edge/pull/16), and [example PR for tensorflow-housing-app](https://github.com/grdryn/ai-edge/pull/17)), and also with the new JSON example GitOps repo file that this PR adds (see [example PR](https://github.com/grdryn/ai-edge/pull/15)). > >## Merge criteria: > > > >- [x] The commits are squashed in a cohesive manner and have meaningful messages. >- [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). >- [x] The developer has manually tested the changes and verified that the changes work > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=opendatahub-io%2Fai-edge). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 4 months ago

@grdryn: This pull request references RHOAIENG-3128 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to [this](https://github.com/opendatahub-io/ai-edge/pull/236): > >JIRA: https://issues.redhat.com/browse/RHOAIENG-3128 > >## Description > >The intention of this change is to make the GitOps update pipeline both more robust and more flexible. Before this change, this pipeline was very tied to the way we do things in the example GitOps repositories that we've got in this repo, and users didn't have the ability to deviate from that very much. > >This proposed change allows them to specify a script to be run, with the intention that they'll specify one or more `yq` commands, to update whichever files in their GitOps repository need to be updated with the new image details. `IMAGE_NAME` and `IMAGE_DIGEST` for the new image will be available as environment variables to the script at runtime. See the examples for example commands/scripts. > >This PR also fixes a minor bug where the `git-clone` task would just checkout the default branch (i.e. `main`) despite what the user had specified in the PipelineRun using the `gitRepoBaseBranch` parameter. While this was previously causing the PR to be correctly created against the expected target branch, the change wasn't being made on top of that branch. This would cause issues if the file(s) that the GitOps Update pipeline was targeting didn't exist on the default branch (would cause pipeline error), or were different enough from the version of the file(s) on the target branch (would cause an unmergeable PR). > >**Breaking change:** >The `imageReferenceFilePath` parameter has been removed, and the `yq-script` has been added. In a previous iteration of this PR, the `yq-script` had a default value, but this has been removed now, since it referred to the `imageReferenceFilePath` param. Instead, each of the example PipelineRuns contain a `yq-script` value equivalent to the previous behaviour. > >## How Has This Been Tested? > > > >~This has just been tested with the existing examples in this repo so far. I will update here when I've tried it with different scenarios, per the acceptance criteria in the Jira issue.~ >This has been tested with the pre-existing examples in the repo here (see [example PR for bike-rental-app](https://github.com/grdryn/ai-edge/pull/16), and [example PR for tensorflow-housing-app](https://github.com/grdryn/ai-edge/pull/17)), and also with the new JSON example GitOps repo file that this PR adds (see [example PR](https://github.com/grdryn/ai-edge/pull/15)). > >## Merge criteria: > > > >- [x] The commits are squashed in a cohesive manner and have meaningful messages. >- [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). >- [x] The developer has manually tested the changes and verified that the changes work > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=opendatahub-io%2Fai-edge). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
MarianMacik commented 4 months ago

Looks good with the new changes too!