Add custom metalk8s_network.routes execution module #3352

alexandre-allard · 2021-05-04T17:43:28Z

Component: salt, tests

Context:
We rely on the Salt network.routes execution module to retrieve the routes during pre-check to ensure a route exists for a specific destination (or at least a default route).
The issue is that this module is bugged and fails when there is a blackhole route defined, which is our case, since Calico defines one.

Summary:
We implement a basic version of the network.routes module, fixing the blackhole issue and only keeping what we really need.

Acceptance criteria:
Unit tests are OK and salt-call metalk8s_network.routes returns the list of routes without crashing when a blackhole route is defined and there is no net-tools package installed

Closes: #3349

bert-e · 2021-05-04T17:43:31Z

Hello alexandre-allard-scality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2021-05-04T17:43:38Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/3349-network-routes-mod into
development/2.8
w/2.9/bugfix/3349-network-routes-mod will be merged into development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2021-05-04T17:43:39Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

salt/_modules/metalk8s_network.py

bert-e · 2021-05-05T06:53:06Z

History mismatch

Merge commit #ec287557a01076cc2790dd2ae5bb8b67af39828e on the integration branch
w/2.9/bugfix/3349-network-routes-mod is merging a branch which is neither the current
branch bugfix/3349-network-routes-mod nor the development branch
development/2.9.

It is likely due to a rebase of the branch bugfix/3349-network-routes-mod and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

salt/_modules/metalk8s_network.py

This module is a simplified version of Salt network.routes, only handling IPv4 routes for Linux dists using `ip` utility. The goal of the module is to workaround a bug in the original one when there is a blackhole route defined (see: saltstack/salt#60126) Once this bug is fixed upstream, we can remove it. Refs: #3349

We use a custom module because the upstream one is bugged. Refs: #3349

alexandre-allard · 2021-05-05T07:09:01Z

/reset

bert-e · 2021-05-05T07:09:08Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2021-05-05T07:09:16Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/3349-network-routes-mod into
development/2.8
w/2.9/bugfix/3349-network-routes-mod will be merged into development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

bert-e · 2021-05-05T07:09:17Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

alexandre-allard · 2021-05-05T07:27:29Z

/approve

Also fix metalk8s_checks.routes_exists test since it was mocking network.routes module. Refs: #3349

bert-e · 2021-05-05T07:34:30Z

History mismatch

Merge commit #06afdf92deaea6a57f45412458e7992cb84c1602 on the integration branch
w/2.9/bugfix/3349-network-routes-mod is merging a branch which is neither the current
branch bugfix/3349-network-routes-mod nor the development branch
development/2.9.

It is likely due to a rebase of the branch bugfix/3349-network-routes-mod and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

The following options are set: approve

alexandre-allard · 2021-05-05T07:35:22Z

@gdemonet @TeddyAndrieux Forgot to update the tests according to the changes requested... you can re-approve now :)

alexandre-allard · 2021-05-05T07:35:28Z

/reset

bert-e · 2021-05-05T07:35:35Z

Reset complete

I have successfully deleted this pull request's integration branches.

The following options are set: approve

bert-e · 2021-05-05T07:35:42Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/3349-network-routes-mod into
development/2.8
w/2.9/bugfix/3349-network-routes-mod will be merged into development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: approve

bert-e · 2021-05-05T07:35:43Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

gdemonet · 2021-05-05T08:10:33Z

salt/tests/unit/modules/files/test_metalk8s_network.yaml

+    result: []
+  # 5. Unsupported type or format
+  - ip_route_output: |-
+      banana route via foo


I've been greatly inspired by your tests. :)

bert-e · 2021-05-05T08:52:17Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.8
✔️ development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2021-05-05T10:16:45Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.8
✔️ development/2.9

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7

Please check the status of the associated issue None.

Goodbye alexandre-allard-scality.

alexandre-allard requested a review from a team as a code owner May 4, 2021 17:43

gdemonet reviewed May 4, 2021

View reviewed changes

salt/_modules/metalk8s_network.py Outdated Show resolved Hide resolved

alexandre-allard force-pushed the bugfix/3349-network-routes-mod branch from 335e63b to 985f87b Compare May 5, 2021 06:53

gdemonet reviewed May 5, 2021

View reviewed changes

salt/_modules/metalk8s_network.py Show resolved Hide resolved

alexandre-allard added 2 commits May 5, 2021 09:08

salt: Use custom metalk8s_network.routes module

e4f9054

We use a custom module because the upstream one is bugged. Refs: #3349

alexandre-allard force-pushed the bugfix/3349-network-routes-mod branch from 985f87b to 06afdf9 Compare May 5, 2021 07:08

TeddyAndrieux previously approved these changes May 5, 2021

View reviewed changes

gdemonet previously approved these changes May 5, 2021

View reviewed changes

salt,tests: Add unit test for metalk8s_network.routes

aefb577

Also fix metalk8s_checks.routes_exists test since it was mocking network.routes module. Refs: #3349

alexandre-allard dismissed stale reviews from gdemonet and TeddyAndrieux via aefb577 May 5, 2021 07:34

alexandre-allard force-pushed the bugfix/3349-network-routes-mod branch from 06afdf9 to aefb577 Compare May 5, 2021 07:34

gdemonet approved these changes May 5, 2021

View reviewed changes

TeddyAndrieux approved these changes May 5, 2021

View reviewed changes

bert-e merged commit 481a21e into development/2.8 May 5, 2021

bert-e deleted the bugfix/3349-network-routes-mod branch May 5, 2021 10:16

alexandre-allard linked an issue May 5, 2021 that may be closed by this pull request

Service CIDR pre-check fails if blackhole routes exist #3349

Closed

alexandre-allard mentioned this pull request May 5, 2021

Service CIDR pre-check fails if blackhole routes exist #3349

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom metalk8s_network.routes execution module #3352

Add custom metalk8s_network.routes execution module #3352

alexandre-allard commented May 4, 2021

bert-e commented May 4, 2021

bert-e commented May 4, 2021

bert-e commented May 4, 2021

bert-e commented May 5, 2021

alexandre-allard commented May 5, 2021

bert-e commented May 5, 2021

bert-e commented May 5, 2021

bert-e commented May 5, 2021

alexandre-allard commented May 5, 2021

bert-e commented May 5, 2021

alexandre-allard commented May 5, 2021

alexandre-allard commented May 5, 2021

bert-e commented May 5, 2021

bert-e commented May 5, 2021

bert-e commented May 5, 2021

gdemonet May 5, 2021

TeddyAndrieux May 5, 2021

alexandre-allard May 5, 2021

bert-e commented May 5, 2021

bert-e commented May 5, 2021

Add custom metalk8s_network.routes execution module #3352

Add custom metalk8s_network.routes execution module #3352

Conversation

alexandre-allard commented May 4, 2021

bert-e commented May 4, 2021

Hello alexandre-allard-scality,

bert-e commented May 4, 2021

Integration data created

bert-e commented May 4, 2021

Waiting for approval

bert-e commented May 5, 2021

History mismatch

alexandre-allard commented May 5, 2021

bert-e commented May 5, 2021

Reset complete

bert-e commented May 5, 2021

Integration data created

bert-e commented May 5, 2021

Waiting for approval

alexandre-allard commented May 5, 2021

bert-e commented May 5, 2021

History mismatch

alexandre-allard commented May 5, 2021

alexandre-allard commented May 5, 2021

bert-e commented May 5, 2021

Reset complete

bert-e commented May 5, 2021

Integration data created

bert-e commented May 5, 2021

Waiting for approval

gdemonet May 5, 2021

Choose a reason for hiding this comment

TeddyAndrieux May 5, 2021

Choose a reason for hiding this comment

alexandre-allard May 5, 2021

Choose a reason for hiding this comment

bert-e commented May 5, 2021

In the queue

bert-e commented May 5, 2021