Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: align manifest images #131

Conversation

tarilabs
Copy link
Member

@tarilabs tarilabs commented Oct 5, 2024

No description provided.

Signed-off-by: Matteo Mortari <[email protected]>
@openshift-ci openshift-ci bot requested review from dhirajsb and rareddy October 5, 2024 13:40
Copy link

openshift-ci bot commented Oct 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tarilabs

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:

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

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.58%. Comparing base (d9a2e20) to head (69f71bc).

Additional details and impacted files
@@               Coverage Diff               @@
##           release/v0.2.8     #131   +/-   ##
===============================================
  Coverage           77.58%   77.58%           
===============================================
  Files                  24       24           
  Lines                2159     2159           
  Branches              145      145           
===============================================
  Hits                 1675     1675           
  Misses                287      287           
  Partials              197      197           

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

@tarilabs tarilabs added the lgtm label Oct 5, 2024
@tarilabs
Copy link
Member Author

tarilabs commented Oct 5, 2024

note: build job successfull (the one doing unit, integration) and same for python testing.
job build-and-test-image (dry-run the operator successfully deploy an instance of MR) failed which currently expect a Model Registry Operator branch, but that can only happen after a release of operator, which wants the Model Registry image.

...seems to me a circular dependency issue.

I propose to move the GHA job build-and-test-image to the Model Registry Operator,
or alternatively to amend the current GHA job build-and-test-image to use the main branche from MR Operator.

cc @Al-Pragliola what is your pov here?

since the job build-and-test-image is mainly to dry-run the operator deploys the image, proceeding anyway with the release, since testing is done both upstream and unit, integration testing with the other GHA jobs

@tarilabs tarilabs merged commit 700d3eb into opendatahub-io:release/v0.2.8 Oct 5, 2024
12 of 14 checks passed
@Al-Pragliola
Copy link

Absolutely, I agree with you @tarilabs that the MR operator is dependent on the MR. So I think it is wrong to do the integration test here in MR, I would also move it to the operator. If we want to avoid the scenario where we release MR v0.x.y and then see that it breaks with the MR operator and we have to re-release or bump to MR v0.x.y+1, we can include the use of RCs in both repos and then when everything is green do the release.

@tarilabs
Copy link
Member Author

tarilabs commented Oct 7, 2024

this is a great idea @Al-Pragliola thank you for the suggestion.

As discussed offline, for the next time, I will do branching "at once" in both repos. Since we release from main, this is possibly a simple way to test this.

Can always opt for RC strategy when we have some changes that requires attention in the interaction between the operator and MR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants