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

Use arm32v7|arm64v8 images instead of the deprecated armhf|aarch64 image organizations #50602

Merged
merged 4 commits into from Sep 3, 2017
Merged

Use arm32v7|arm64v8 images instead of the deprecated armhf|aarch64 image organizations #50602

merged 4 commits into from Sep 3, 2017

Conversation

dixudx
Copy link
Member

@dixudx dixudx commented Aug 14, 2017 •
edited by luxas

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #50601

Special notes for your reviewer:
/assign @ixdy @jbeda @zmerlynn
Release note:

Use arm32v7|arm64v8 images instead of the deprecated armhf|aarch64 image organizations

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 14, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @dixudx. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 14, 2017
luxas
luxas reviewed Aug 14, 2017
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

LGTM, but I think there are more aarch64's lying around in our codebase. For example in test/images/*/BASEIMAGE

cc @ixdy @mkumatag

@luxas luxas added this to the v1.8 milestone Aug 14, 2017
@luxas
Copy link
Member

luxas commented Aug 14, 2017

/ok-to-test
/assign

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 14, 2017
@mkumatag
Copy link
Member

@luxas we don't have aarch64 images anymore in test/images folder, while multi-arch porting I've migrated all the images to arm64v8 repo.

mkumatag
mkumatag approved these changes Aug 14, 2017
View reviewed changes
Copy link
Member

@mkumatag mkumatag left a comment

Choose a reason for hiding this comment

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

LGTM

@mkumatag
Copy link
Member

mkumatag commented Aug 14, 2017 •
edited

similar way we can get rid of armhf images as well, we can migrate to arm32v7

@dixudx
Copy link
Member Author

dixudx commented Aug 14, 2017

@mkumatag Good. I would like to take this.

@dixudx
Copy link
Member Author

dixudx commented Aug 14, 2017

@mkumatag @luxas PTAL #50616. Thanks.

luxas
luxas approved these changes Aug 14, 2017
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

The candidates for bumping the version number for here are:

  • build/debian-base/Makefile
  • cluster/addons/addon-manager/Makefile

@ixdy WDYT? Should we bump those or not?

@mkumatag
Copy link
Member

@luxas I vote for updating version as we are changing the baseimage and older image is a stale one which is older than 3 months.

@dixudx
Copy link
Member Author

dixudx commented Aug 14, 2017

/retest

@dixudx
Copy link
Member Author

dixudx commented Aug 15, 2017

/test pull-kubernetes-e2e-gce-etcd3

@ixdy
Copy link
Member

ixdy commented Aug 18, 2017

yeah, we should probably bump the version tag since the base image is changing, not just being refreshed.

@ixdy
Copy link
Member

ixdy commented Aug 18, 2017

(it's probably worth combining #50616 with this so we only need to bump the tag once)

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 19, 2017
@dixudx
Copy link
Member Author

dixudx commented Aug 19, 2017

ping @luxas @mkumatag @ixdy

I bumped new version in this PR. For other related PRs #50616, #50597 should get merged before this PR.

PTAL. Thanks.

This was referenced Aug 19, 2017
Closed
Merged
@dixudx
Copy link
Member Author

dixudx commented Aug 20, 2017

/retest

@luxas
Copy link
Member

luxas commented Sep 1, 2017

/retest

@ixdy
Copy link
Member

ixdy commented Sep 1, 2017

that's probably fine. we need better OWNERS in cluster/images IMO.

@luxas
Copy link
Member

luxas commented Sep 1, 2017

@ixdy if you feel you've got the names of those persons, please send a PR with such an addition...

@luxas
Copy link
Member

luxas commented Sep 1, 2017

@ixdy in the meantime though, can you approve this?

@ixdy
Copy link
Member

ixdy commented Sep 1, 2017

yeah, I am building and pushing images right now.

@luxas luxas added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2017
@ixdy
Copy link
Member

ixdy commented Sep 2, 2017

debian-base and debian-iptables images have been pushed. working on hyperkube-base now.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2017
@ixdy ixdy closed this Sep 2, 2017
@ixdy ixdy reopened this Sep 2, 2017
@ixdy
Copy link
Member

ixdy commented Sep 2, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2017
@ixdy
Copy link
Member

ixdy commented Sep 2, 2017

(why is that "close and comment" button so close to the other one?)

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dixudx, ixdy, luxas
We suggest the following additional approver: jbeda

Assign the PR to them by writing /assign @jbeda in a comment when ready.

Associated issue: 50601

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@dixudx
Copy link
Member Author

dixudx commented Sep 2, 2017

/test pull-kubernetes-e2e-kops-aws

@dixudx
Copy link
Member Author

dixudx commented Sep 2, 2017

/test pull-kubernetes-kubemark-e2e-gce

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@dixudx
Copy link
Member Author

dixudx commented Sep 2, 2017

/retest

@luxas
Copy link
Member

luxas commented Sep 2, 2017

/retest

@ixdy I feel you, have happened to me as well

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@luxas
Copy link
Member

luxas commented Sep 3, 2017

/retest

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 50602, 51561, 51703, 51748, 49142)

@k8s-github-robot k8s-github-robot merged commit 5d72d5c into kubernetes:master Sep 3, 2017
@dixudx dixudx deleted the user_arm64v8_instead_aarch64 branch September 3, 2017 08:12
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 3, 2017 •
edited

@dixudx: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 4d842cb link /test pull-kubernetes-e2e-kops-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@mkumatag
Copy link
Member

@ixdy can you push latest test images?

@dixudx dixudx mentioned this pull request Oct 13, 2017
Closed
@ixdy
Copy link
Member

ixdy commented Oct 13, 2017

I think I pushed these but never commented here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luxas luxas luxas approved these changes

@ixdy ixdy ixdy left review comments

@mkumatag mkumatag mkumatag approved these changes

Assignees

@jbeda jbeda

@zmerlynn zmerlynn

@luxas luxas

@ixdy ixdy

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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.8
Development

Successfully merging this pull request may close these issues.

The aarch64 organization is deprecated in favor of the more-specific arm64v8 organization
9 participants
@dixudx @k8s-ci-robot @luxas @mkumatag @ixdy @k8s-github-robot @fejta-bot @jbeda @zmerlynn