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

PR1 - To install and upgrade noobaa version for running the tests #8517

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

achouhan09
Copy link
Member

@achouhan09 achouhan09 commented Nov 11, 2024

Explain the changes

  1. This PR is mainly to install and upgrade noobaa version from master to latest.

Testing Instructions:

  1. In PR2 we will the the required tests.

@achouhan09 achouhan09 marked this pull request as draft November 11, 2024 16:19
@achouhan09 achouhan09 force-pushed the upgrade-tests branch 8 times, most recently from 1032b01 to 15a23fc Compare November 18, 2024 16:08
@achouhan09 achouhan09 marked this pull request as ready for review November 19, 2024 07:42
@achouhan09 achouhan09 changed the title Added upgrade tests PR1 - To install and upgrade noobaa version for running the tests Nov 19, 2024
@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 27, 2024
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/upgrade-tests.yaml Outdated Show resolved Hide resolved
@achouhan09 achouhan09 force-pushed the upgrade-tests branch 4 times, most recently from 9fcd6af to 7808b68 Compare December 30, 2024 15:36
@achouhan09 achouhan09 requested a review from shirady December 31, 2024 06:13
- name: Build noobaa (latest)
run: |
cd ./noobaa-core
make noobaa NOOBAA_TAG=noobaa-core:upgrade-tests
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are on master branch, then the image of noobaa-core:master and noobaa-core:upgrade-tests are the same? (maybe I missed something - why should we continue with the job?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Here, the flow is like in step 1 we are building noobaa core from master/input branch for installation of noobaa and in step 4 we are building noobaa core using PR code to upgrade the noobaa.

--db-resources='{ "limits": {"cpu": "200m","memory": "2G"}, "requests": {"cpu": "200m","memory": "2G"}}' \
--core-resources='{ "limits": {"cpu": "200m","memory": "1G"}, "requests": {"cpu": "200m","memory": "1G"}}' \
--endpoint-resources='{ "limits": {"cpu": "200m","memory": "1G"}, "requests": {"cpu": "200m","memory": "1G"}}' \
--noobaa-image='noobaa-core:${{ env.BRANCH_NAME || 'master' }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

In the step you wrote: "Install noobaa system (from master)" - did you mean master in the core repo, right? Then why --noobaa-image='noobaa-core:${{ env.BRANCH_NAME || 'master' }}'? I think it should be noobaa-core:upgrade-tests (which according to the steps represents "Build noobaa (latest)".
Are you sure the system should be installed in the latest version?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have also added pull request referece in latest build (in step 4) in my last commit, it would work I think.

- name: Upgrade noobaa to latest
run: |
cd ./noobaa-operator
VERSION=$(go run cmd/version/main.go)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm suggesting to add that it is the operator repo version, so it won't be confusing.

Suggested change
VERSION=$(go run cmd/version/main.go)
OPERATOR_VERSION=$(go run cmd/version/main.go)

run: |
cd ./noobaa-operator
VERSION=$(go run cmd/version/main.go)
./build/_output/bin/noobaa-operator upgrade --noobaa-image='noobaa-core:upgrade-tests' --operator-image='noobaa/noobaa-operator:$VERSION'
Copy link
Contributor

Choose a reason for hiding this comment

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

In case you would accept the suggestion then it needs to be changed also here:

Suggested change
./build/_output/bin/noobaa-operator upgrade --noobaa-image='noobaa-core:upgrade-tests' --operator-image='noobaa/noobaa-operator:$VERSION'
./build/_output/bin/noobaa-operator upgrade --noobaa-image='noobaa-core:upgrade-tests' --operator-image='noobaa/noobaa-operator:$OPERATOR_VERSION'

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

Successfully merging this pull request may close these issues.

2 participants