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

Add proxy env vars to pods #278

Merged
merged 1 commit into from Apr 6, 2020
Merged

Conversation

jmontleon
Copy link
Collaborator

@jmontleon jmontleon commented Mar 26, 2020

This configure proxy env vars if they are set on the operator or in the MigrationController CR.

It does not yet address the issue of certificates.

For each of the following check the box when you have verified either:

  • the changes have been made for each applicable version
  • no changes are required for the item

Affected versions:

  • Latest
  • 1.1
  • 1.0

The CSV is responsible in OLM installs for

  • Operator permissions
  • Operator deployment
  • Operand permissions
  • CRDs

The operator.yml is responsible in non-OLM installs for

  • Operator permissions
  • Operator deployment

The ansible role is responsible in non-OLM installs for:

  • Operand permissions
  • CRDs

The ansible role is always responsible for:

  • Operand deployment

If this PR updates a release or replaces channel

  • I created a new z release directory in deploy/olm-catalog/konveyor-operator
  • I updated channels in the konveyor-operator.package.yaml
  • I created a new release directory in deploy/non-olm
  • I created or updated the major.minor link in deploy/non-olm
  • Updated the .github/pull_request_template.md Affected versions list

Copy link
Contributor

@eriknelson eriknelson left a comment

Choose a reason for hiding this comment

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

LGTM as far as I understand the problem. I'm unsure if the UI server will need to use this or not, @jmontleon do we want to continue testing this before merging or merge this and continue to update as we find problems? This PR is pretty harmless.

@jmontleon
Copy link
Collaborator Author

Yes. I'd like to do some research and take a stab at implementing some approach for getting a CA Bundle in the containers as well. Might have it today depending how other things go and what I find and come up with.

@jmontleon
Copy link
Collaborator Author

If this proves beneficial without the CA work for a customer we can always merge it and come back to CA support later too.

@eriknelson
Copy link
Contributor

If this proves beneficial without the CA work for a customer we can always merge it and come back to CA support later too.

Agreed, the custom proxy CA is potentially an edge case and the env vars alone could be useful to have as incremental support.

@jmontleon jmontleon marked this pull request as ready for review April 2, 2020 18:28
@jmontleon jmontleon changed the title Configure proxy on velero, restic, and controller if required Add proxy env vars to pods Apr 2, 2020
Copy link
Contributor

@eriknelson eriknelson left a comment

Choose a reason for hiding this comment

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

Tested this in conjunction with a bunch of other patched images we identified that needed changes for the limited support (ignoring the custom proxy CA), and I had success staging and migrating an S2I application from a 3.x -> 4.x disconnected environment requiring a proxy for outside comms in the QE repro environment. I added some background info describing the problems with the other components on the controller PR.

This is definitely useful enough in its current form for us to merge and shoot to release initial support for 1.1.2. The known improvements I'd like to follow up with are:

  • I need to add another PR with some docs that describe the overall setup, main concern is 3.x since it requires manual config. I'm not sure that the operator itself needs the proxy vars in its deployment, or if we just need to add the proxy settings to the CR for the operands, @jmontleon want to catch up with you about that.
  • The custom proxy CA case we've talked about. Also need to talk to Shawn about that one to get a better idea about what we need to cover.

Needs to be merged in conjunction with:
migtools/mig-controller#473 - Controller support
vmware-tanzu/velero-plugin-for-aws#37 - This code should also be complete. Need to coordinate with @dymurray on this one to figure out how to make sure this gets upstreamed, and if we want to rebase on top of the upstream plugin, or if we want to commit this directly to our fork so we can release this with 1.1.2 in the immediate term and then rebase on the upstream plugin for 1.2.

@eriknelson eriknelson merged commit 5b704a5 into migtools:master Apr 6, 2020
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