Ping external ip to verify connectivity #202

qinqon · 2019-10-04T10:28:12Z

Signed-off-by: Quique Llorente [email protected]

qinqon · 2019-10-04T11:11:25Z

I am installing "iputils" but we may want to use golang tools like https://godoc.org/github.com/sparrc/go-ping both options are going to increase the size of Dockerfile

phoracek · 2019-10-04T11:12:57Z

@qinqon at least we can use the container for network debugging when needed :)

pkg/helper/client.go

phoracek · 2019-10-04T12:15:57Z

deploy/operator.yaml

@@ -55,6 +55,11 @@ spec:
                configMapKeyRef:
                  name: nmstate-config
                  key: interfaces_filter
+            - name: CONNECTIVITY_CHECK_ENDPOINT


If I was a user I would have no idea what the endpoint means. Maybe connectivity_check_ip_target?

But since it can be domain name, maybe just connectivity_check_target?

ack, is good name too.

phoracek

We should have a test for this.

SchSeba

I am not sure this is a good solution for disconnected environment.

can we just save the environment default gateway before the change make the change and check it again?
also we can add a check that we can still query the k8s api.

phoracek · 2019-10-05T11:23:58Z

@SchSeba the API ping would come later. We want to start with this since we work on single node environment, so access to API server is not that hard. However y the idea with default gateway sounds great. Less configuration variables.

qinqon · 2019-10-07T10:16:08Z

@SchSeba @phoracek I like the idea of use the default gateway as the connectivity target, but want to keep the environment variable for testing purposes is that ok ?

qinqon · 2019-10-08T08:44:17Z

@phoracek @SchSeba in case we can modify the default gw with a bad value, we don't need the config to test it.

SchSeba · 2019-10-08T08:56:29Z

@qinqon for a test you can do it.

just run the ip route add command on the host to change the default one. (you need to remove the correct one and the create a new one)

Then just return the one you deleted on the afterEach

phoracek · 2019-10-08T09:33:29Z

@qinqon @SchSeba we can configure the route via nmstate ;) https://nmstate.github.io/examples.html#route

qinqon · 2019-10-08T10:02:44Z

@SchSeba my idea is to try to do it first using nmstate, if this renders impossible we can hack it at node.

phoracek · 2019-10-08T11:48:49Z

pkg/helper/client.go

@@ -159,12 +167,26 @@ func UpdateCurrentState(client client.Client, nodeNetworkState *nmstatev1alpha1.
 	return nil
 }

+func ping(target string) error {


It may be interesting to log the output of the ping too. Don't you think?

I want to see CI passing and then will cover this.

qinqon · 2019-10-08T13:28:38Z

@SchSeba @phoracek looks like we can change default gw with nmstate but they have an issue and it's only possible changing eth0 to static ip address, but at least is working.

pkg/helper/client.go

test/e2e/rollback_test.go

test/e2e/utils.go

test/e2e/rollback_test.go

pkg/helper/client.go

In case it fails it will rollback nmstate changes. Signed-off-by: Quique Llorente <[email protected]>

Remove Brad and Toni from reviewers

qinqon force-pushed the rollback_lost_connectivity branch 2 times, most recently from efec0a2 to 09cecb9 Compare October 4, 2019 11:09

qinqon requested review from phoracek and SchSeba October 4, 2019 11:10

phoracek reviewed Oct 4, 2019

View reviewed changes

pkg/helper/client.go Outdated Show resolved Hide resolved

qinqon force-pushed the rollback_lost_connectivity branch from 09cecb9 to 262f9fc Compare October 4, 2019 12:05

qinqon changed the title ~~wip: ping external ip to verify connectivity~~ Ping external ip to verify connectivity Oct 4, 2019

qinqon requested a review from phoracek October 4, 2019 12:06

phoracek requested changes Oct 4, 2019

View reviewed changes

qinqon force-pushed the rollback_lost_connectivity branch 2 times, most recently from fc0e1cf to beaf1e6 Compare October 4, 2019 12:35

phoracek requested changes Oct 4, 2019

View reviewed changes

SchSeba reviewed Oct 5, 2019

View reviewed changes

qinqon force-pushed the rollback_lost_connectivity branch 3 times, most recently from cac643a to e5ee2f7 Compare October 8, 2019 08:37

phoracek reviewed Oct 8, 2019

View reviewed changes

qinqon force-pushed the rollback_lost_connectivity branch 2 times, most recently from d728669 to 3de0ab8 Compare October 8, 2019 13:21

phoracek requested changes Oct 8, 2019

View reviewed changes

qinqon force-pushed the rollback_lost_connectivity branch from 3de0ab8 to d5e6366 Compare October 8, 2019 13:55

qinqon requested review from SchSeba and phoracek October 8, 2019 14:03

phoracek requested changes Oct 8, 2019

View reviewed changes

test/e2e/utils.go Show resolved Hide resolved

test/e2e/rollback_test.go Outdated Show resolved Hide resolved

qinqon force-pushed the rollback_lost_connectivity branch from d5e6366 to bcfcd4e Compare October 8, 2019 14:20

qinqon requested a review from phoracek October 8, 2019 14:21

qinqon force-pushed the rollback_lost_connectivity branch 2 times, most recently from 281c436 to 440e52a Compare October 8, 2019 14:48

phoracek requested changes Oct 8, 2019

View reviewed changes

pkg/helper/client.go Outdated Show resolved Hide resolved

pkg/helper/client.go Outdated Show resolved Hide resolved

pkg/helper/client.go Outdated Show resolved Hide resolved

qinqon force-pushed the rollback_lost_connectivity branch from 440e52a to 58fedf1 Compare October 8, 2019 15:05

qinqon requested a review from phoracek October 8, 2019 15:05

phoracek previously approved these changes Oct 8, 2019

View reviewed changes

Ping default gw from withing the cluster to verify connectivity

5a6a1fe

In case it fails it will rollback nmstate changes. Signed-off-by: Quique Llorente <[email protected]>

qinqon dismissed phoracek’s stale review via 5a6a1fe October 8, 2019 15:32

qinqon force-pushed the rollback_lost_connectivity branch from 35bcfad to 5a6a1fe Compare October 8, 2019 15:32

phoracek approved these changes Oct 8, 2019

View reviewed changes

phoracek merged commit 8e8df75 into nmstate:master Oct 8, 2019

cybertron pushed a commit to cybertron/kubernetes-nmstate that referenced this pull request Aug 27, 2021

Merge pull request nmstate#202 from cybertron/owners-cleanup

cb1952b

Remove Brad and Toni from reviewers

kubevirt-bot mentioned this pull request Aug 27, 2021

Bug 1998208: Rebase to upstream 0.52.4 #814

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ping external ip to verify connectivity #202

Ping external ip to verify connectivity #202

qinqon commented Oct 4, 2019

qinqon commented Oct 4, 2019

phoracek commented Oct 4, 2019

phoracek Oct 4, 2019

phoracek Oct 4, 2019

qinqon Oct 4, 2019

phoracek left a comment

SchSeba left a comment

phoracek commented Oct 5, 2019

qinqon commented Oct 7, 2019 •

edited

Loading

qinqon commented Oct 8, 2019

SchSeba commented Oct 8, 2019

phoracek commented Oct 8, 2019

qinqon commented Oct 8, 2019 •

edited

Loading

phoracek Oct 8, 2019

qinqon Oct 8, 2019

qinqon commented Oct 8, 2019

Ping external ip to verify connectivity #202

Ping external ip to verify connectivity #202

Conversation

qinqon commented Oct 4, 2019

qinqon commented Oct 4, 2019

phoracek commented Oct 4, 2019

phoracek Oct 4, 2019

Choose a reason for hiding this comment

phoracek Oct 4, 2019

Choose a reason for hiding this comment

qinqon Oct 4, 2019

Choose a reason for hiding this comment

phoracek left a comment

Choose a reason for hiding this comment

SchSeba left a comment

Choose a reason for hiding this comment

phoracek commented Oct 5, 2019

qinqon commented Oct 7, 2019 • edited Loading

qinqon commented Oct 8, 2019

SchSeba commented Oct 8, 2019

phoracek commented Oct 8, 2019

qinqon commented Oct 8, 2019 • edited Loading

phoracek Oct 8, 2019

Choose a reason for hiding this comment

qinqon Oct 8, 2019

Choose a reason for hiding this comment

qinqon commented Oct 8, 2019

qinqon commented Oct 7, 2019 •

edited

Loading

qinqon commented Oct 8, 2019 •

edited

Loading