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.

Sign up for GitHub

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

Jump to bottom

🌱 E2E tests CentOS support #225

Merged
merged 1 commit into from
Jul 8, 2021
Merged

🌱 E2E tests CentOS support #225

merged 1 commit into from
Jul 8, 2021

Conversation

wgslr
Copy link
Member

@wgslr wgslr commented Jun 10, 2021

What this PR does / why we need it:

This PR makes the e2e tests pass on a CentOS host by adding a modified template file.

This PR builds on top of #218, so the diff is currently larger than just the relevant changes.

@wgslr
Copy link
Member Author

wgslr commented Jun 10, 2021

Needs to be merged after #218
/hold

@metal3-io-bot metal3-io-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 10, 2021
@wgslr wgslr force-pushed the e2e-centos-support/wgslr branch 2 times, most recently from 95af2b5 to fb1d144 Compare June 11, 2021 13:17
@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2021
@wgslr wgslr changed the base branch from release-0.4 to master June 18, 2021 07:08
@metal3-io-bot metal3-io-bot added needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 18, 2021
@metal3-io-bot metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2021
@wgslr
Copy link
Member Author

wgslr commented Jun 30, 2021

/unhold
/test-v1a5-e2e
/test-integration

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 30, 2021
@wgslr
Copy link
Member Author

wgslr commented Jun 30, 2021

/test-v1a5-e2e

1 similar comment
@wgslr
Copy link
Member Author

wgslr commented Jul 2, 2021

/test-v1a5-e2e

@wgslr wgslr force-pushed the e2e-centos-support/wgslr branch from 66588ed to 2e23eab Compare July 6, 2021 08:21
@wgslr
Copy link
Member Author

wgslr commented Jul 6, 2021

/test-v1a5-e2e
/test-integration

namnx228
namnx228 reviewed Jul 6, 2021
View reviewed changes
Copy link
Member

@namnx228 namnx228 left a comment

Choose a reason for hiding this comment

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

LGTM

fmuyassarov
fmuyassarov reviewed Jul 6, 2021
View reviewed changes
Makefile Outdated Show resolved Hide resolved
furkatgofurov7
furkatgofurov7 suggested changes Jul 6, 2021
View reviewed changes
hack/e2e/environment.sh Outdated Show resolved Hide resolved
hack/e2e/environment.sh Outdated Show resolved Hide resolved
hack/e2e/environment.sh Outdated Show resolved Hide resolved
templates/test/cluster-template-prow-ha.yaml
@@ -204,7 +204,7 @@ spec:
image:
checksum: ${IMAGE_RAW_CHECKSUM}
checksumType: ${IMAGE_CHECKSUM_TYPE}
format: ${IMAGE_FORMAT}
format: raw
Copy link
Member

Choose a reason for hiding this comment

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

why are we reverting it back? if that is the case, you must get rid of the IMAGE_FORMAT var from environment.sh

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, I realized that I need to remove IMAGE_FORMAT from environment.sh as well, just didn't push it yet.
As for why: since I use the variables IMAGE_RAW_URL and IMAGE_RAW_CHECKSUM, the image can be always assumed to use the raw format. Changing this will require modifying the template here anyway, so I think it's better not to give a false sense of the image format being changeable by environment.sh alone.

templates/test/cluster-template-prow-ha.yaml
@@ -300,7 +300,7 @@ spec:
image:
checksum: ${IMAGE_RAW_CHECKSUM}
checksumType: ${IMAGE_CHECKSUM_TYPE}
format: ${IMAGE_FORMAT}
format: raw
Copy link
Member

Choose a reason for hiding this comment

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

same

templates/test/cluster-template-prow-ha-centos.yaml
image:
checksum: ${IMAGE_RAW_CHECKSUM}
checksumType: ${IMAGE_CHECKSUM_TYPE}
format: raw
Copy link
Member

Choose a reason for hiding this comment

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

why not to parameterize?

templates/test/cluster-template-prow-ha-centos.yaml
image:
checksum: ${IMAGE_RAW_CHECKSUM}
checksumType: ${IMAGE_CHECKSUM_TYPE}
format: raw
Copy link
Member

Choose a reason for hiding this comment

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

same

@furkatgofurov7
Copy link
Member

/hold

Let's wait for e2e centos job to be in place to test these changes in CI.

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2021
@wgslr
Copy link
Member Author

wgslr commented Jul 6, 2021

/test-v1a5-centos-e2e

@wgslr wgslr force-pushed the e2e-centos-support/wgslr branch from cbef6a2 to 63cbd6b Compare July 6, 2021 14:11
@wgslr
Copy link
Member Author

wgslr commented Jul 6, 2021

Centos CI job is now present
/unhold
/test-v1a5-centos-e2e
/test-v1a5-e2e
/test-integration

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2021
furkatgofurov7
furkatgofurov7 reviewed Jul 7, 2021
View reviewed changes
Copy link
Member

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

/approve

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: furkatgofurov7, lentzi90

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 7, 2021
@furkatgofurov7
Copy link
Member

/cc @Xenwar @jan-est @fmuyassarov

@jan-est
Copy link
Contributor

jan-est commented Jul 8, 2021

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2021
@metal3-io-bot metal3-io-bot merged commit 4fbc775 into metal3-io:master Jul 8, 2021
Xenwar
Xenwar reviewed Jul 8, 2021
View reviewed changes
Makefile Show resolved Hide resolved
@kashifest kashifest deleted the e2e-centos-support/wgslr branch December 9, 2022 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@namnx228 namnx228 namnx228 left review comments

@Xenwar Xenwar Xenwar left review comments

@furkatgofurov7 furkatgofurov7 furkatgofurov7 requested changes

@lentzi90 lentzi90 lentzi90 approved these changes

@fmuyassarov fmuyassarov Awaiting requested review from fmuyassarov

@jan-est jan-est Awaiting requested review from jan-est

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@wgslr @furkatgofurov7 @metal3-io-bot @jan-est @namnx228 @lentzi90 @Xenwar @fmuyassarov