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

automatically detect node ip during install for noproxy #1186

Merged

Conversation

laverya
Copy link
Member

@laverya laverya commented Sep 17, 2024

What this PR does / why we need it:

With no no-proxy set:

no-proxy was not set, adding the default interface's subnet "172.17.0.0/16" to the no-proxy
? Set the Admin Console password:

With an insufficient no-proxy set:

The provided no-proxy "10.0.0.0/24" does not cover the local IP "172.17.0.2", adding the default interface's subnet "172.17.0.0/16" to the no-proxy we will use
? Set the Admin Console password:

When joining a node that is not included in the cluster's no-proxy:

no-proxy config "localhost,127.0.0.1,.cluster.local,.svc,10.0.0.2,10.244.0.0/16,10.96.0.0/12" does not allow access to local IP "10.0.0.3"

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Installations using a proxy will automatically have the default interface's subnet supplied as a no-proxy if the provided no-proxy does not cover the instance IP address.

Does this PR require documentation?

Copy link

github-actions bot commented Sep 17, 2024

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-eac4573" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-eac4573?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

pkg/netutils/ips.go Outdated Show resolved Hide resolved
return proxy
}

func combineNoProxySuppliedValuesAndDefaults(c *cli.Context, proxy *ecv1beta1.ProxySpec) {
Copy link
Member

Choose a reason for hiding this comment

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

name is too long 😆, but i guess it's fine. i would've gone with something like injectNoProxyDefaults or something.

Suggested change
func combineNoProxySuppliedValuesAndDefaults(c *cli.Context, proxy *ecv1beta1.ProxySpec) {
func combineNoProxySuppliedValuesAndDefaults(c *cli.Context, proxy *ecv1beta1.ProxySpec) {

@@ -86,6 +86,7 @@ Additionally, it includes a Registry when deployed in air gap mode.
```

1. In the Vendor Portal, create and download a license that is assigned to the channel.
We recommend storing this license in the `local-dev/` directory, as it is gitignored and not otherwise used by the CI.
Copy link
Member

@sgalsaleh sgalsaleh Sep 18, 2024

Choose a reason for hiding this comment

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

take it or leave it:

Suggested change
We recommend storing this license in the `local-dev/` directory, as it is gitignored and not otherwise used by the CI.
We recommend storing this license in the `local-dev/` directory that is dedicated to storing local development artifacts, as it is gitignored and not otherwise used by the CI.

sgalsaleh
sgalsaleh previously approved these changes Sep 18, 2024
@laverya laverya enabled auto-merge (squash) September 18, 2024 15:42
Copy link
Member

@ajp-io ajp-io left a comment

Choose a reason for hiding this comment

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

let me know if I missed any output I should review

cmd/embedded-cluster/proxy.go Outdated Show resolved Hide resolved
cmd/embedded-cluster/proxy.go Outdated Show resolved Hide resolved
Co-authored-by: Alex Parker <7272359+ajp-io@users.noreply.github.com>
@laverya laverya enabled auto-merge (squash) September 19, 2024 14:42
@laverya laverya merged commit 058761e into main Sep 19, 2024
59 checks passed
@laverya laverya deleted the laverya/sc-111539/automatically-detect-node-ip-during-install branch September 19, 2024 15:07
@sgalsaleh sgalsaleh restored the laverya/sc-111539/automatically-detect-node-ip-during-install branch September 19, 2024 15:37
@sgalsaleh sgalsaleh deleted the laverya/sc-111539/automatically-detect-node-ip-during-install branch September 19, 2024 16:12
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