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

Make image-user test images multi-arch #300

Merged
merged 1 commit into from
May 8, 2018
Merged

Make image-user test images multi-arch #300

merged 1 commit into from
May 8, 2018

Conversation

nitkon
Copy link
Contributor

@nitkon nitkon commented May 5, 2018 •
edited
Loading

Fixes #287
This fixes Issue 287 partially. Here "docker push" is used instead of
"gcloud docker -- push" as its depreciated now. Reference

Signed-off-by: Nitesh Konkar [email protected]

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 5, 2018
@nitkon nitkon mentioned this pull request May 5, 2018
Closed
feiskyer
feiskyer reviewed May 7, 2018
View reviewed changes
images/image-user/Makefile Outdated
.PHONY: all test-image-user-uid test-image-user-username test-image-user-uid-group test-image-user-username-group push-manifest

REGISTRY = gcr.io/cri-tools
ALL_ARCH = amd64 ppc64le
Copy link
Member

Choose a reason for hiding this comment

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

Add all arches that kubernetes has already supported? e.g. amd64 arm arm64 ppc64le s390x

feiskyer
feiskyer reviewed May 7, 2018
View reviewed changes
images/image-user/Dockerfile
@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

FROM busybox
ARG ARCH
FROM $ARCH/busybox
Copy link
Member

Choose a reason for hiding this comment

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

seems there are no arm/busybox and arm64/busybox images?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@feiskyer : Should we use arm32v7 and arm64v8 ? Reference

Copy link
Member

@mkumatag mkumatag May 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

yes we have to use arm32v7 and arm64v8 but these does not match the architecture value what you get via go env

@nitkon
Copy link
Contributor Author

nitkon commented May 7, 2018

@feiskyer : @mkumatag : Addressed your comments.

feiskyer
feiskyer reviewed May 8, 2018
View reviewed changes
images/image-user/Makefile Outdated
@@ -12,22 +12,34 @@
# See the License for the specific language governing permissions and
# limitations under the License.

include ../../hack/make-rules/Makefile.manifest
include ../../hack/make-rules/Baseimage
Copy link
Member

Choose a reason for hiding this comment

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

s/Baseimage/BASEIMAGES

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@feiskyer : Thnx! Fixed.

@feiskyer
Copy link
Member

feiskyer commented May 8, 2018

thanks

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2018
@feiskyer feiskyer merged commit 6ae7b25 into kubernetes-sigs:master May 8, 2018
@feiskyer
Copy link
Member

feiskyer commented May 8, 2018

@Random-Liu Could you build and push those new images?

@Random-Liu
Copy link
Contributor

Random-Liu commented May 16, 2018 •
edited
Loading

I got this:

make push-manifest
curl -sSL https://github.com/estesp/manifest-tool/releases/download/v0.7.0/manifest-tool-linux-amd64 > /tmp/tmp.sbbBxWuS49/manifest-tool
chmod +x /tmp/tmp.sbbBxWuS49/manifest-tool
manifest-tool push from-args --platforms linux/amd64,linux/arm,linux/arm64,linux/ppc64le,linux/s390x --template gcr.io/cri-tools/test-image-user-uid-ARCH:latest --target gcr.io/cri-tools/test-image-user-uid:latest; manifest-tool push from-args --platforms linux/amd64,linux/arm,linux/arm64,linux/ppc64le,linux/s390x --template gcr.io/cri-tools/test-image-user-username-ARCH:latest --target gcr.io/cri-tools/test-image-user-username:latest; manifest-tool push from-args --platforms linux/amd64,linux/arm,linux/arm64,linux/ppc64le,linux/s390x --template gcr.io/cri-tools/test-image-user-uid-group-ARCH:latest --target gcr.io/cri-tools/test-image-user-uid-group:latest; manifest-tool push from-args --platforms linux/amd64,linux/arm,linux/arm64,linux/ppc64le,linux/s390x --template gcr.io/cri-tools/test-image-user-username-group-ARCH:latest --target gcr.io/cri-tools/test-image-user-username-group:latest;
FATA[0008] Couldn't push manifests referenced in our manifest list: Referenced manifest push unsuccessful: response 401: 401 Unauthorized 
FATA[0008] Couldn't push manifests referenced in our manifest list: Referenced manifest push unsuccessful: response 401: 401 Unauthorized 
FATA[0007] Couldn't push manifests referenced in our manifest list: Referenced manifest push unsuccessful: response 401: 401 Unauthorized 
FATA[0007] Couldn't push manifests referenced in our manifest list: Referenced manifest push unsuccessful: response 401: 401 Unauthorized 
Makefile:45: recipe for target 'push-manifest' failed
make: *** [push-manifest] Error 1

And we should add push-manifest into all target.

@mkumatag
Copy link
Member

It depends on the .docker/config file entry with proper credentials to login gcr.io. So you need to check whether docker command is able to push the image to gcr or not.

@nitkon
Copy link
Contributor Author

nitkon commented May 16, 2018

@Random-Liu : I have addressed your comment in the following PR

@Random-Liu
Copy link
Contributor

It depends on the .docker/config file entry with proper credentials to login gcr.io. So you need to check whether docker command is able to push the image to gcr or not.

I did gcloud credential thing, let me try again tonight.

@nitkon
Copy link
Contributor Author

nitkon commented May 22, 2018

Hi @Random-Liu, Did you get a chance to build and push these images again.

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

@mkumatag mkumatag mkumatag left review comments

@feiskyer feiskyer feiskyer left review comments

Assignees

@feiskyer feiskyer

Labels
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make test-images multiarch
5 participants
@nitkon @feiskyer @Random-Liu @mkumatag @k8s-ci-robot