Skip to content

Add proxy config to standalone container registry#1090

Merged
ajdecon merged 2 commits intoNVIDIA:masterfrom
nuka137:add-proxy-config-to-standalone-container-registry
Jan 18, 2022
Merged

Add proxy config to standalone container registry#1090
ajdecon merged 2 commits intoNVIDIA:masterfrom
nuka137:add-proxy-config-to-standalone-container-registry

Conversation

@nuka137
Copy link
Copy Markdown
Contributor

@nuka137 nuka137 commented Jan 10, 2022

Current standalone container registry can not work on the proxy environment.
This PR enables us to specify the proxy configurations for standalone container registry.

@ajdecon
Copy link
Copy Markdown
Contributor

ajdecon commented Jan 11, 2022

@nuka137 : This looks reasonable to me!

Can you please sign off on your commit as detailed in our CONTRIBUTING.md doc? And then I can merge the PR.

Signed-off-by: Atsushi Nukariya <a.nukariya@jp.fujitsu.com>
@nuka137 nuka137 force-pushed the add-proxy-config-to-standalone-container-registry branch from 0286d3e to cd425ff Compare January 11, 2022 23:58
@nuka137
Copy link
Copy Markdown
Contributor Author

nuka137 commented Jan 12, 2022

@ajdecon

Recommitted about previous patch.
Could you check it?

Copy link
Copy Markdown
Contributor

@ajdecon ajdecon left a comment

Choose a reason for hiding this comment

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

This actually appears to fail in the default case (without a proxy). If the variable isn't defined, Ansible won't run.

We use a different pattern in other tasks (example) that looks like this:

- name: apt install pciutils
  apt: name=pciutils update_cache=yes
  when: ansible_os_family == 'Debian'
  environment: "{{ proxy_env if proxy_env is defined else {} }}"

And then define proxy_env if needed:

# proxy_env:
      # http_proxy: '{{ http_proxy }}'
      # https_proxy: '{{ https_proxy }}'
      # no_proxy: '{{ no_proxy }}'

Can you convert this to either use the same proxy_env variable, or a similar pattern that works when no proxy is present?

Signed-off-by: Atsushi Nukariya <a.nukariya@jp.fujitsu.com>
@nuka137
Copy link
Copy Markdown
Contributor Author

nuka137 commented Jan 16, 2022

@ajdecon

Thanks. I fixed it.

Copy link
Copy Markdown
Contributor

@ajdecon ajdecon left a comment

Choose a reason for hiding this comment

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

LGTM

@ajdecon ajdecon merged commit 6eb1ca4 into NVIDIA:master Jan 18, 2022
@nuka137 nuka137 deleted the add-proxy-config-to-standalone-container-registry branch January 18, 2022 21:11
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.

2 participants