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

build providers: honour http proxy settings for snapd #3251

Merged
merged 2 commits into from Aug 20, 2020
Merged

Conversation

cjp256
Copy link
Contributor

@cjp256 cjp256 commented Aug 18, 2020

Configure snapd with the proxy settings configured by the user
for http_proxy and https_proxy. Do this on each launch so
that any changes in configuration passed are applied.

Signed-off-by: Chris Patterson chris.patterson@canonical.com

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

@cjp256
Copy link
Contributor Author

cjp256 commented Aug 18, 2020

I believe there is a bit of chicken and egg for the case(s) where snap injection isn't supported...

@cjp256
Copy link
Contributor Author

cjp256 commented Aug 18, 2020

Hrmm, naturally this was going to break the spread test that passed in http proxy...

@@ -259,6 +258,19 @@ def launch_instance(self) -> None:
# what is on the host
self._setup_snapcraft()

# Configure snapd to use http/https proxy.
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about putting this in it's own method and do some "super" tricks in multipass and lxd to minimize the test spread?

Copy link
Collaborator

@sergiusens sergiusens left a comment

Choose a reason for hiding this comment

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

Looks good, just added a minor comment to improve tests

@@ -747,6 +746,36 @@ def test_lifecycle(
hide_output=False,
instance_name="snapcraft-project-name",
),
call(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well this whole test case appears to be about testing the whole thing... the LXD version uses asserts_has_calls(), but also enumerates the whole thing (minus these new changes because it never failed). If the purpose of the test is to simply check that it has any calls, and that each call has the proper provider-specific prefix commands, I can do that. Otherwise, I'm not sure?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was trying to transform it into that for changes not to have this ripple effect. It is true that it is not the case for everything today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed updates to clean that up in areas touched 👍

Chris Patterson added 2 commits August 20, 2020 19:28
Configure snapd with the proxy settings configured by the user
for http_proxy and https_proxy.  Do this on each launch so
that any changes in configuration passed are applied.

Update env-passthrough tests to use developer debug and
enable experimental extensions now that http proxy affects
snapd operation.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
Just verify the command prefixes match what is expected for
each environment.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
@sergiusens sergiusens merged commit fbd93aa into master Aug 20, 2020
@sergiusens sergiusens deleted the snapd-proxy branch August 20, 2020 21:01
abitrolly pushed a commit to abitrolly/snapcraft that referenced this pull request Mar 31, 2021
Configure snapd with the proxy settings configured by the user
for http_proxy and https_proxy.  Do this on each launch so
that any changes in configuration passed are applied.

Update env-passthrough tests to use developer debug and
enable experimental extensions now that http proxy affects
snapd operation.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants