Skip to content

docs: update workflow and how we use two branchs for ODH and downstream - #1582

Merged
openshift-merge-bot[bot] merged 4 commits into
opendatahub-io:mainfrom
zdtsw-forking:jira_498
Jan 30, 2025
Merged

openshift-merge-bot[bot] merged 4 commits into
opendatahub-io:mainfrom
zdtsw-forking:jira_498

Conversation

@zdtsw

@zdtsw zdtsw commented Jan 29, 2025

Copy link
Copy Markdown
Member

Description

https://issues.redhat.com/browse/RHOAIENG-498

How Has This Been Tested?

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • 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

@zdtsw zdtsw added the documentation Improvements or additions to documentation label Jan 29, 2025
@openshift-ci
openshift-ci Bot requested review from StevenTobin and lphiri January 29, 2025 10:15
@zdtsw
zdtsw requested review from ugiordan and removed request for StevenTobin and lphiri January 29, 2025 10:15
@zdtsw zdtsw changed the title docs: update workflow and how we use two branchs for ODH and downstream [WIP] docs: update workflow and how we use two branchs for ODH and downstream Jan 29, 2025
@codecov

codecov Bot commented Jan 29, 2025 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.94%. Comparing base (cb2baa9) to head (fc4165c).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1582   +/-   ##
=======================================
  Coverage   19.94%   19.94%           
=======================================
  Files         158      158           
  Lines       10799    10799           
=======================================
  Hits         2154     2154           
  Misses       8415     8415           
  Partials      230      230           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zdtsw zdtsw changed the title [WIP] docs: update workflow and how we use two branchs for ODH and downstream docs: update workflow and how we use two branchs for ODH and downstream Jan 29, 2025
Comment thread .github/workflows/linter.yaml Outdated
branches:
- main
- incubation
- rhoai

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really a doc

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, put in #1593 for change

Comment thread CONTRIBUTING.md Outdated
3. **Operator Bundle or Dockerfile Updates:** If changes affect the operator bundle or Dockerfile, sync the changes in the `rhods-cpaas-midstream` repo in addition to downstream PR.

Follow the same [PR Guide](#pull-requests) for creating and reviewing downstream PRs.
detail see (./docs/sync_code.md#basci-workflow-for-operator)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a typo, basic

Comment thread docs/sync_code.md
Start by creating a branch from "main" branch.

2. Ensure local tests pass:
Run unit and e2e tests, and ensure they pass before submitting a pull request (PR) targeting "main" branch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will we switch to jira-XXX-name branch naming at the same time?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the branch is created in the users' fork git repo, can be named any way they would like to. i do not think we need suggest that part.

Comment thread docs/sync_code.md


3. Review and merge PR:
Once the PR has passed github checks and is reviewed and approved, the PR author should merge it into "main" branch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is author really involved? Was it bot or maintainer?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my experience:
either i need to enable auto-merge myself or i need to manually click "merge" or someone helps to click "merge/auto-merge" for me, prow does not help in a 100% reliable way.
until it is fully automated without so many "exceptions", i would prefer the author of the PR follow it till the end.
if the author is a community user, then might have problem to "merge", maintainer can be tagged for such cases. this could also happens when these PR need to get into downstream.

Comment thread docs/sync_code.md
6. Automated sync:
Automation will sync changes to the RHDS rhods-operator on a daily basis.

# PR labels

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we allowed to set labels?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping that the /label command could be used for that, but it seems that it's a static list of labels specific to openshift that are allowed for that. Maybe something could be added to the config in openshift/release for it, not sure 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think manitainers should have permission to set label.
at least i used labels a lot both for my PR and the PR i was asked to review or triage.

also i do not know how it will be with these prow bot, when we move to konflux, do we still need prow, will /cmd still work etc.
if we do not want to depend on an external system, e.g prow, native feature of GH like create/add labels should always work, (i hope)

Comment thread docs/sync_code.md
5. Merge sync PR:
After the sync PR has passed github checks and is reviewed and approved, the PR author should merge it into "rhoai" branch.

6. Automated sync:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure it should be in the ODH repo. Does not affect any related workflow according to the description.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put it here only as information.
if everyone is clear about this, because i got asked many times in the past, i can remove it

Comment thread docs/sync_code.md
3. Review and merge PR:
Once the PR has passed github checks and is reviewed and approved, the PR author should merge it into "main" branch.

4. Create a downstream sync PR:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double checking, it's now part of process for any author.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be different.
if we would like, we can have two different cases:
-for community user
-for the most active users of this git repo normally the maintainer/platform team members.

i drafted this PR purely for the later case, but then the responsibilty for who should get such sync PR to downstream is not defined. we will need another discussion internally for that.
but that can be done in a follow up PR once we know more. wdyt?

@grdryn

grdryn commented Jan 29, 2025

Copy link
Copy Markdown
Member

/label rhoai

@openshift-ci

openshift-ci Bot commented Jan 29, 2025

Copy link
Copy Markdown

@grdryn: The label(s) /label rhoai cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label rhoai

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Comment thread docs/sync_code.md Outdated
Comment thread docs/sync_code.md
6. Automated sync:
Automation will sync changes to the RHDS rhods-operator on a daily basis.

# PR labels

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping that the /label command could be used for that, but it seems that it's a static list of labels specific to openshift that are allowed for that. Maybe something could be added to the config in openshift/release for it, not sure 🤔

zdtsw and others added 4 commits January 30, 2025 08:56
- add new doc for how workflow, label on PR, sync process
- update branch away from incubatin in github action and reference

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Gerard Ryan <git@grdryn.xyz>
Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Signed-off-by: Wen Zhou <wenzhou@redhat.com>
@openshift-ci

openshift-ci Bot commented Jan 30, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ugiordan, ykaliuta

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit da75d42 into opendatahub-io:main Jan 30, 2025
zdtsw added a commit to zdtsw-forking/opendatahub-operator that referenced this pull request Feb 3, 2025
…am (opendatahub-io#1582)

* docs: update workflow and how we use two branchs for ODH and downstream

- add new doc for how workflow, label on PR, sync process
- update branch away from incubatin in github action and reference

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Update docs/sync_code.md

Co-authored-by: Gerard Ryan <git@grdryn.xyz>

* revert: branch related test config

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: typo

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Gerard Ryan <git@grdryn.xyz>
(cherry picked from commit da75d42)
zdtsw added a commit to zdtsw-forking/opendatahub-operator that referenced this pull request Feb 10, 2025
…am (opendatahub-io#1582)

* docs: update workflow and how we use two branchs for ODH and downstream

- add new doc for how workflow, label on PR, sync process
- update branch away from incubatin in github action and reference

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Update docs/sync_code.md

Co-authored-by: Gerard Ryan <git@grdryn.xyz>

* revert: branch related test config

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: typo

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Gerard Ryan <git@grdryn.xyz>
(cherry picked from commit da75d42)
openshift-merge-bot Bot pushed a commit that referenced this pull request Feb 10, 2025
…am (#1582) (#1636)

* docs: update workflow and how we use two branchs for ODH and downstream

- add new doc for how workflow, label on PR, sync process
- update branch away from incubatin in github action and reference

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Update docs/sync_code.md

Co-authored-by: Gerard Ryan <git@grdryn.xyz>

* revert: branch related test config

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: typo

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Gerard Ryan <git@grdryn.xyz>
(cherry picked from commit da75d42)
openshift-merge-bot Bot pushed a commit that referenced this pull request Feb 10, 2025
* chore: cleanup kustomize config (#1497)

- suspend too many WARNING lines from "make bundle"
- remove sample config for components and monitoring

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit 4fe5bed)

* chore: add more rules in golint (#1487)

- add rules for our API components and service which force alias to be named with suffix ctrl

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit 32d75a8)

* update: clean up old code before refactor (#1494)

* update: clean up old code before refactor

- remove reference we explicilty check on odhdashboardconfig and kserve
- remove customized config on controller QPS
- remove RemoveLabel which is not called anywhere

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* update:

- remove function for upgrade cleanup introduced before 2.8
- remove flags to main: operator namespace and application namespace
  ( operator namespace is passing as env variable, app namespace is done by DSCI )

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: Auth CR is cluster-scoped no need to set namespace for request

- application namespace is removed from reconciler of dsci

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: wrong code for upgrade path + rebase error

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit b0a0f87)

* docs: update workflow and how we use two branchs for ODH and downstream (#1582)

* docs: update workflow and how we use two branchs for ODH and downstream

- add new doc for how workflow, label on PR, sync process
- update branch away from incubatin in github action and reference

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* Update docs/sync_code.md

Co-authored-by: Gerard Ryan <git@grdryn.xyz>

* revert: branch related test config

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: typo

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Gerard Ryan <git@grdryn.xyz>
(cherry picked from commit da75d42)

* update: platform team (#1583)

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit 46d8ba2)

* Remove Sara4994 from OWNER_ALIASES (#1598)

(cherry picked from commit bf8a6e2)

* github: add branch into GHA for tests (#1593)

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit f4c932b)

* apis: DSCInitialization: annotate ApplicationsNamespace with omitempty (#1585)

Jira: https://issues.redhat.com/browse/RHOAIENG-18931

Api sets default value to opendatahub, but without "omitempty" in
case of operator automatically creates
DSCI (upgrade.go:CreateDefaultDSCI), the resulting object contains
applicationsNamespace: "" and it's not defaulted by the api server
causing then problems like

```
Namespace "" is invalid: metadata.name: Required value: name or generateName is required
```

The problem was triggered by removing default value for command line
switch in the cleanup.

Fixes: b0a0f87 ("update: clean up old code before refactor (#1494)")

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
(cherry picked from commit 768539c)

* regen bundle (#1602)

(cherry picked from commit 2384ada)

* test: prolong e2e in rhoai for 50mins

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix:default monitoring namespace

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
Co-authored-by: Ajay Jaganathan <36824134+AjayJagan@users.noreply.github.com>
Co-authored-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Co-authored-by: Luca Burgazzoli <lburgazzoli@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved documentation Improvements or additions to documentation lgtm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants