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

Set DNS trusted servers using FelixConfig instead of FELIX_ env var #2700

Merged
merged 4 commits into from
Jun 30, 2023

Conversation

nelljerram
Copy link
Member

@nelljerram nelljerram commented Jun 22, 2023

For Rancher and OpenShift the operator needs to set a non-default value for DNS trusted servers, and it was previously doing that by setting a FELIX_ env var in the node daemonset. But that makes it impossible for any further tweaks to that using FelixConfiguration - because FelixConfiguration settings are always overridden by FELIX_ env vars.

To allow later tweaks, change the operator to create or patch FelixConfiguration instead.

References:
https://tigera.atlassian.net/browse/RS-897
https://tigera.atlassian.net/browse/EV-3193

Description

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

For Rancher and OpenShift the operator needs to set a non-default value for DNS trusted servers, and
it was previously doing that by setting a FELIX_ env var in the node daemonset.  But that makes it
impossible for any further tweaks to that using FelixConfiguration - because FelixConfiguration
settings are always overridden by FELIX_ env vars.

To allow later tweaks, change the operator to create or patch FelixConfiguration instead.

References:
https://tigera.atlassian.net/browse/RS-897
https://tigera.atlassian.net/browse/EV-3193
@nelljerram
Copy link
Member Author

I think the CI failure is unrelated. Will wait for review feedback, in case there are other issues to address, before hitting Rerun.

Copy link
Member

@caseydavenport caseydavenport left a comment

Choose a reason for hiding this comment

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

LGTM - I really like the common helper for performing FC patches. Thanks @nelljerram!

@nelljerram nelljerram merged commit 65d04a4 into master Jun 30, 2023
@nelljerram nelljerram deleted the dns-trusted-servers branch June 30, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants