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

Update to build and push multi-arch manifests #3143

Merged
merged 6 commits into from Mar 8, 2019

Conversation

jeefy
Copy link
Member

@jeefy jeefy commented Jul 15, 2018

This allows us to publish v2.2 registry manifests allowing end-users to consume a single container image rather than specifying their architecture. This will directly address and resolve several issues (including #3137 and #2811)

Example:
docker pull kubernetes-dashboard
Instead of
docker pull kubernetes-dashbaord-amd64

  • Update travis-ci.yml to use a newer version of Docker
  • Update build/Dockerfile to use a newer version of Docker
  • pushToDocker now creates and pushes manifests

Tested and verified on both Docker Hub and GCR (Only had access to a private repo sorry)

If/When this is accepted, there will be additional work updating other repos and documentation to use the Manifest image (kubernetes-dashboard) instead of individual images (kubernetes-dashboard-amd64)

Any questions or comments are appreciated!

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 15, 2018
@jeefy jeefy force-pushed the multiarch-container branch 2 times, most recently from a0ca1b7 to 90029ff Compare July 16, 2018 00:41
@codecov
Copy link

codecov bot commented Jul 16, 2018

Codecov Report

Merging #3143 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3143      +/-   ##
==========================================
+ Coverage      48%   48.02%   +0.02%     
==========================================
  Files         165      165              
  Lines        7960     7960              
  Branches       24       24              
==========================================
+ Hits         3821     3823       +2     
+ Misses       3885     3882       -3     
- Partials      254      255       +1
Impacted Files Coverage Δ
...p/backend/integration/metric/common/aggregation.go 89.09% <0%> (-1.82%) ⬇️
src/app/backend/sync/secret.go 73.26% <0%> (+2.97%) ⬆️

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 f51ac63...6120951. Read the comment docs.

@jeefy
Copy link
Member Author

jeefy commented Jul 16, 2018

/assign @PeWu

@austbot
Copy link
Contributor

austbot commented Jul 19, 2018

Can you pull the install docker and go into some shell script files and then run them in docker file or Travis that way?

@floreks
Copy link
Member

floreks commented Jul 19, 2018

I have one concern regarding this change. As it requires edge docker client with manually enabled experimental flag, it might be a problem for users to use this instead of the current installation methods. At first, we should keep it only as an addition to current deployment methods and not make it the default.

@jeefy
Copy link
Member Author

jeefy commented Jul 19, 2018

It only requires the experimental flag to push a manifest to a registry.

Pulling is supported in the stable version of Docker (and has been for some time, early 2017 I believe)

You could pull jeefy/kubernetes-dashboard and it will pull the correct image based on your arch.

On the SIG call today, we decided to hold this back from the next release and look to merge it in after.

@floreks
Copy link
Member

floreks commented Jul 24, 2018

Great, if this will work without additional changes to docker then I am ok with it.

@jeefy
Copy link
Member Author

jeefy commented Jul 24, 2018

/hold

Awesome! I think we're waiting till after the next release to merge this in.

Once the angular migration is done we can merge this and update everything surrounding it in future PRs.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 24, 2018
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 13, 2018
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 14, 2018
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 14, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 13, 2018
@jeefy
Copy link
Member Author

jeefy commented Dec 13, 2018

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 13, 2018
@maciaszczykm
Copy link
Member

Once the angular migration is done we can merge this and update everything surrounding it in future PRs.

@jeefy Are we planning to something about it?

@jeefy
Copy link
Member Author

jeefy commented Jan 3, 2019 via email

@jeefy jeefy mentioned this pull request Jan 10, 2019
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2019
@jeefy
Copy link
Member Author

jeefy commented Jan 13, 2019

@floreks @maciaszczykm PTAL, I think it's in a good place.

@jeefy
Copy link
Member Author

jeefy commented Jan 13, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2019
@maciaszczykm
Copy link
Member

@jeefy Can you solve conflicts?

@mkumatag
Copy link
Member

mkumatag commented Mar 8, 2019

what else pending part of this pr? can we merge this?

@maciaszczykm
Copy link
Member

@mkumatag Nothing :)

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 8, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jeefy, maciaszczykm

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

@k8s-ci-robot k8s-ci-robot merged commit 6edc1da into kubernetes:master Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants