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

feat(examples): upgrade rules_docker to 0.13.0 #1496

Merged
merged 1 commit into from
Apr 15, 2020

Conversation

Toxicable
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@Toxicable
Copy link
Author

Toxicable commented Dec 21, 2019

Getting this one in CircleCI

2019/12/21 05:23:32 Image pull was unsuccessful: reading image "gcr.io/google-appengine/debian9@sha256:c05b781371f75d1bd7a199bc83de177173cc80c98dbfb6c1ef7075757addece4": .dockercfg: $HOME is not defined

It looks like rules_docker is looking for $HOME/.dockercfg ref: https://github.com/bazelbuild/rules_docker/blob/a5a0a766a145698cd7c32bd149bd638a278bac8a/container/pull.bzl#L34
What docker container does CircleCI, could we adjust it to get $HOME defined?
Or should we push back at rules_docker to not look for this file if $HOME isn't defined?
This dosen't happen in BazelCI

examples/angular fails to build locally due to the below error, but I havn't seen it in CI, is that a problem?

fabianwiles@Fabians-MacBook-Air angular % npx bazel run --platforms=@build_bazel_rules_nodejs//toolchains/node:linux_amd64 //src:nodejs_image
INFO: Invocation ID: 2fcaf538-0960-4a1a-b75d-bd4a764c126a
ERROR: error loading package '': in /private/var/tmp/_bazel_fabianwiles/f5889d45a4ee08f70c0e782c50e402a9/external/npm_bazel_typescript/index.b
zl: in /private/var/tmp/_bazel_fabianwiles/f5889d45a4ee08f70c0e782c50e402a9/external/npm_bazel_typescript/internal/build_defs.bzl: in /private/var/tmp/_bazel_fabianwiles/f5889d45a4ee08f70c0e782c50e402a9/external/npm_bazel_typescript/internal/common/compilation.bzl: Unable to load file '@build_bazel_rules_nodejs//:declaration_provider.bzl': file doesn't exist

@alexeagle
Copy link
Collaborator

I don't know whether it's correct for rules_docker to look for $HOME
but it looks like we just need to provide a value for docker_client_config so it doesn't try to look there?

@Toxicable
Copy link
Author

So if I try work around it by defining HOME just for that command I get this failure
https://circleci.com/gh/bazelbuild/rules_nodejs/37598?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Is there some other way we could work around this for now?

@Toxicable Toxicable force-pushed the docker_v0.13.0 branch 2 times, most recently from 3af541b to d6828b3 Compare January 26, 2020 19:59
@Toxicable
Copy link
Author

not really sure how to proceed with this one, is there somewhere we could just define HOME in our environment to work around the above issue?

@alexeagle
Copy link
Collaborator

I tried a couple of things, looks like just having the integration test runner define a $HOME env variable is sufficient.
added to the commit

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

nice cleanup 🌯

@alexeagle alexeagle force-pushed the docker_v0.13.0 branch 3 times, most recently from b6fb970 to 84c83e1 Compare April 14, 2020 14:35
@alexeagle
Copy link
Collaborator

Currently stuck because the environment for a bazel-in-bazel is missing something that git needs to do operations...

@alexeagle
Copy link
Collaborator

I confirmed that it passes with SSH_AUTH_SOCK=/tmp/circleci-090238626/ssh_auth_sock so need to plumb that through

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@gregmagolan
Copy link
Collaborator

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Also drop the patches we needed for older versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants