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

wire up e2e hab pkg download tests into expeditor #7104

Merged
merged 8 commits into from Oct 28, 2019

Conversation

@jeremymv2
Copy link
Contributor

jeremymv2 commented Oct 24, 2019

This PR accomplishes the following:

  • wires up e2e tests for hab pkg download
  • covers bugs/features recently merged for the new download command
  • finally, adds some minor cleanups and enhancements to the test/end-to-end/test_pkg_download.sh script
@markan
markan approved these changes Oct 24, 2019
Copy link
Contributor

markan left a comment

LGTM

@@ -278,6 +278,17 @@ steps:
- BUILD_PKG_TARGET=x86_64-windows
- BUILDKITE_AGENT_ACCESS_TOKEN

- label: "[:linux: test_pkg_download]"

This comment has been minimized.

Copy link
@markan

markan Oct 24, 2019

Contributor

Eyeballing the expeditor test timings, a great deal of time is setting up the environment compared to the actual test execution time. As we add more of these tests this might not scale super well; we might want to just add a batch script to run all / most of the tests in the end_to_end directory in a single test env.

This comment has been minimized.

Copy link
@mwrock

mwrock Oct 24, 2019

Contributor

what I have noticed is the bulk of time can be spent in docker pull but sometimes the image is already there. I don't understand why sometimes it is there and sometimes not but ensuring the docker images are on the test VMs could significantly improve performance.

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 25, 2019

Contributor

Just to be clear, this whole "parallelization via Buildkite" approach was always intended at a near-term expedient. Eventually, we need to set up some kind of actual test runner code infrastructure to manage these tests.

@@ -278,6 +278,17 @@ steps:
- BUILD_PKG_TARGET=x86_64-windows
- BUILDKITE_AGENT_ACCESS_TOKEN

- label: "[:linux: test_pkg_download]"
command:
- .expeditor/scripts/end_to_end/setup_environment.sh DEV

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 25, 2019

Contributor

Need to use "dev" rather than "DEV"; see #7100 for details

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 25, 2019

Author Contributor

Fixed up in 2da2907

jeremymv2 added 5 commits Oct 22, 2019
…bugs

Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2 jeremymv2 force-pushed the jeremymv2/e2e_test_download_bulkupload branch from 3e1a01e to ef20f81 Oct 26, 2019
Copy link
Contributor

christophermaier left a comment

Looks like a good addition... just have a few tiny changes to make it easier to run locally.

CACHE_DIR="test-cache"
IDENT_FILE="ident_file"
HAB_AUTH_TOKEN=${ACCEPTANCE_HAB_AUTH_TOKEN:-${HAB_AUTH_TOKEN}}

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

This doesn't appear to be needed (and also seems to fail if the fallback HAB_AUTH_TOKEN isn't present)

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

I can remove that line. From what I can see, this will mean HAB_AUTH_TOKEN must be set somehow in the pipeline.

I was thinking it would be made available from ACCEPTANCE_HAB_AUTH_TOKEN via expeditor here: https://github.com/habitat-sh/habitat/pull/7104/files#diff-9f269bb242eb760dee635951a1a1226eR382

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

I agree, I don't see this as necessary. I've removed it in this commit

echo "core/gzip" > ${IDENT_FILE}
CMD="$HAB pkg download --download-directory=${CACHE_DIR} --file=${IDENT_FILE}"
CMD="$HAB pkg download --download-directory=${CACHE_DIR} --file=${IDENT_FILE}"

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

When running these locally with the e2e_local.sh script, I get failures because of the HAB_BLDR_CHANNEL environment being set to dev, because all the packages that are being downloaded aren't present in that channel.

Long term, I'm not sure what the best pattern for this should be, since we still want to ensure that all the packages being built in this pipeline are properly resolved. Adding --channel=stable to these download commands would do the trick, as would unset HAB_BLDR_CHANNEL before executing the test functions.

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

Ahh, I wasn't aware of that script. Thank you!

I've added the explicit --channel stable here and unset HAB_BLDR_CHANNEL here

jeremymv2 added 2 commits Oct 28, 2019
Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
#
# Assumptions:
# 1. HAB_AUTH_TOKEN Environment variables is set and valid

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

I don't even think you need a token at all, since all this is downloading stuff (though downloading private packages is perhaps another worthwhile usecase to test in the future!)

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

Agreed. Addressed here f6d3c4b

CACHE_DIR="test-cache"
IDENT_FILE="ident_file"
# we explicitly define --channel where needed in the tests
unset HAB_BLDR_CHANNEL

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

Also just to be clear, I think you only need this, and not also the --channel=stable options. Having both doesn't hurt, of course, but it's a bit of a "belt and suspenders" kind of thing 😄

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

Right, it does seem a little over zealous. I've removed the unset preferring the explicit --channel stable argument here f6d3c4b

I'll let the build go green then merge since it has two approvals. Thank you!

Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2 jeremymv2 merged commit d37ac11 into master Oct 28, 2019
5 checks passed
5 checks passed
DCO This commit has a DCO Signed-off-by
Details
buildkite/habitat-sh-habitat-master-verify Build #3953 passed (28 minutes, 53 seconds)
Details
buildkite/habitat-sh-habitat-master-website Build #1033 passed (52 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details
@chef-expeditor chef-expeditor bot deleted the jeremymv2/e2e_test_download_bulkupload branch Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.