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

set multiarch container as default, disable... #177

Merged
merged 2 commits into from
Jul 15, 2021

Conversation

nickboldt
Copy link
Contributor

@nickboldt nickboldt commented Feb 24, 2021

What does this PR do?

set multiarch container as default, disable single-arch build

Change-Id: I3bf13419e30b11b520b4243dbb21b0cf6374547f
Signed-off-by: nickboldt [email protected]

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

eclipse-che/che#19688 (moving to travis for multiarch)

How to test this PR?

N/A

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@nickboldt
Copy link
Contributor Author

Question:

should we move from

CACHE_IMAGE_FULL: docker.io/cheincubator/che-dashboard:cache

to new repo in existing quay che-incubator org?

CACHE_IMAGE_FULL: quay.io/che-incubator/che-dashboard:cache

That way we're not using as part of the build process, when we're ultimately pushing to Quay anyway.

@github-actions
Copy link

No changes to code coverage between the base branch and the head branch

@che-bot
Copy link
Contributor

che-bot commented Feb 24, 2021

✅ E2E dashboard-next tests succeed 🎉

See Details

  • Jenkins job

  • logs and configs

  • Che dashboard-next image:docker.io/maxura/che-dashboard:177

  • Che server image: docker.io/maxura/che-server:177

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[dashboard-next-ci-test]" to rerun the 'dashboard-next' e2e tests

@github-actions
Copy link

Docker image build succeeded: docker.io/maxura/che-server:che-dashboard-pull-177

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

Nobody complained about removing multi-arch build, it's why it was made as optional due stability reasons.

I see potential value in multiarch build as default but github workflow should be reworked in the corresponding way where each (or some of) architectures are mandatory.
And then I don't see a value in keeping single arch image if Che Server and deployments depends on multiarch.

So, in general I'm OK with proposed multiarch by default, but if it won't be stable again, we will need to review it.

.github/workflows/ci-multiarch.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@che-bot
Copy link
Contributor

che-bot commented Mar 8, 2021

❌ E2E dashboard-next tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[dashboard-next-ci-test]" to rerun the 'dashboard-next' e2e tests

@github-actions
Copy link

github-actions bot commented Mar 8, 2021

No changes to code coverage between the base branch and the head branch

@github-actions
Copy link

github-actions bot commented Mar 8, 2021

Docker image build succeeded: docker.io/maxura/che-server:che-dashboard-pull-177

@dmytro-ndp
Copy link
Contributor

[dashboard-next-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Mar 16, 2021

❌ E2E dashboard-next tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[dashboard-next-ci-test]" to rerun the 'dashboard-next' e2e tests

@dmytro-ndp
Copy link
Contributor

[dashboard-next-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Mar 16, 2021

✅ E2E dashboard-next tests succeed 🎉

See Details

  • Jenkins job

  • logs and configs

  • Che dashboard-next image:quay.io/crw_pr/che-dashboard:177

  • Che server image: quay.io/crw_pr/che-server:177

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[dashboard-next-ci-test]" to rerun the 'dashboard-next' e2e tests

@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #177 (cf5d089) into main (ab26654) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #177   +/-   ##
=======================================
  Coverage   51.51%   51.51%           
=======================================
  Files         143      143           
  Lines        5536     5536           
  Branches      913      913           
=======================================
  Hits         2852     2852           
  Misses       2410     2410           
  Partials      274      274           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab26654...cf5d089. Read the comment docs.

@@ -106,5 +104,5 @@ jobs:
echo "[!] The job 'build-images' didn't provide any outputs. Can't create the manifest list."
exit 1;
fi
docker manifest create ${{ env.IMAGE }}:multiarch-next $AMEND
docker manifest push ${{ env.IMAGE }}:multiarch-next
docker manifest create ${{ env.IMAGE }}:next $AMEND
Copy link
Member

Choose a reason for hiding this comment

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

Well, it also requires adapting PR check not to pass when multiarch build fails.
And it has a drawback... For some reason amd64 build takes 4-6 minutes, while other 35-45 minutes which is not critical but not pleasure when you add a rewording fix in the PR before merging.

@github-actions
Copy link

Docker image build succeeded: docker.io/maxura/che-dashboard:che-dashboard-pull-177

1 similar comment
@github-actions
Copy link

Docker image build succeeded: docker.io/maxura/che-dashboard:che-dashboard-pull-177

@che-bot
Copy link
Contributor

che-bot commented Jul 15, 2021

✅ E2E dashboard tests succeed 🎉

See Details

Test product:

  • Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@sleshchenko sleshchenko merged commit c6aaaa1 into main Jul 15, 2021
@sleshchenko sleshchenko deleted the multiarch-as-default branch July 15, 2021 13:14
@che-bot che-bot added this to the 7.34 milestone Jul 15, 2021
@che-bot
Copy link
Contributor

che-bot commented Jul 20, 2021

❌ E2E dashboard tests failed ❗

See Details

Test product:

  • Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

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

Successfully merging this pull request may close these issues.

5 participants