Make Control Plane Ingress IP configurable #3415

TeddyAndrieux · 2021-06-08T07:59:58Z

Component:

'salt', 'tests'

Context:

Being able to choose which IP will be used for Ingress Control Plane, so that user can manage them self HA of this IP through all Control Plane nodes.

Summary:

Define Control Plane Ingress IP and Endpoint at a single place
Male this Control Plane Ingress IP configurable in the Bootstrap Configuration
Some docs and tests for this

Sees: #2381

NOTE: It's only the first part of Control Plane Ingress HA, as we also want to add an optional MetalLB deployment as part of MetalK8s, so that if it's possible in the user environment HA of Control Plane Ingress can be managed by MetalLB directly.

During bootstrap we need an up to date pillar to have all `mine_function` in the minion pillar before doing a `mine.update`. Since by default `saltutil.refresh_pillar` is asynchrone just add a `wait: true` so that this refresh properly wait for the pillar to be refreshed

bert-e · 2021-06-08T08:00:00Z

Hello teddyandrieux,

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-06-08T08:00:07Z

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-06-08T08:10:18Z

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

+  - cp_ingress_ip_ret: "Error because of banana"
+    raises: true
+    result: "Error because of banana"


This error happens to me all the time, I'm glad it's tested ! 👍

CHANGELOG.md

docs/installation/bootstrap.rst

docs/operation/changing_control_plane_ingress_ip.rst

gdemonet

Few minor changes, most important IMO is the configuration format to build as future-proof as possible.

salt/_modules/metalk8s_network.py

salt/metalk8s/addons/nginx-ingress-control-plane/certs/server.sls

tests/post/steps/test_ui.py

gdemonet · 2021-06-17T14:28:52Z

docs/installation/services.rst

+    root@bootstrap $ kubectl --kubeconfig=/etc/kubernetes/admin.conf \
+      get svc -n metalk8s-ingress ingress-nginx-control-plane-controller \
+      -o=jsonpath='{.spec.externalIPs[0]}{"\n"}'
+    <the ingress control plane IP>


It's just to check that it's the IP we want, you do not like it ?

Most of our commands are getting super verbose (same for running a command in the salt-master)... Not saying it's wrong 😉, just that it'd be nice if we could wrap those calls in some kubectl plugin... 😇

Yess I agree

CHANGELOG.md

docs/installation/bootstrap.rst

gdemonet · 2021-06-17T14:40:03Z

docs/installation/bootstrap.rst

+      will sit on a working master Node. The default value for this
+      Ingress IP is the control plane IP of the Bootstrap node (which means
+      that if you lose the Bootstrap node, you no longer have access to any
+      control plane component).


...through the Ingress controller. But some components also listen on host ports directly, no? Especially K8s API, which is useful to access in case of Bootstrap issues.

Right, but if you lose bootstrap you do not have access to OIDC so access to Kubernetes APIServer (should) do not work as well (if we generated kubeconfig specific to each user 😉)
But agree, I will replace "control plane component" with OIDC and various Control Plane UIs

gdemonet · 2021-06-17T14:41:49Z

docs/operation/changing_control_plane_ingress_ip.rst

+          --kubeconfig=/etc/kubernetes/admin.conf \\
+          $(kubectl --kubeconfig=/etc/kubernetes/admin.conf get pod \\
+          -l "app.kubernetes.io/name=salt-master" \\
+          --namespace=kube-system -o jsonpath='{.items[0].metadata.name}')  \\


Hmm right I could use it 🤔
It will have strange behavior if for whatever reason you have multiple salt-master running, maybe better to keep this one for the moment (so that this procedure may be compatible with future Bootstrap HA 😄 😉 )

Haha good point 👌 We'll have to make these commands simpler anyway, so if we change how we deploy things, we don't have to update it in hundreds of places across our docs.

bert-e · 2021-06-17T14:47:08Z

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 reviewers are expecting changes from the author, or must review again:

@gdemonet

Instead of hardcode Control Plane Ingress Endpoint at several places in the salt states, add a new Salt module to "compute" it and use this salt module everywhere in our salt states

Instead of hardcode Ingress port in tests and use Bootstrap Control Plane IP to compute the Control Plane Ingress Endpoint, retrieve it using the Kubernetes Service

Update the command to retrieve the IP using Kubernetes API and Control Plane Ingress Service to get the external IP

Make the IP used to reach the UI and other Control Plane components configurable from Bootstrap config file Refs: #2381

Move `we are on a multi node cluster` from test registry to conftest, so that it can be used in other tests

gdemonet

(minor typos) 🚀

docs/operation/changing_control_plane_ingress_ip.rst

bert-e · 2021-06-23T08:23:25Z

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:

Add a simple orchestrate and small procedure in the documentation to change the Control Plane Ingress IP

TeddyAndrieux · 2021-06-23T11:10:54Z

/approve

bert-e · 2021-06-23T11:11:08Z

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.10

The following branches will NOT be impacted:

development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

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-06-23T12:42:42Z

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

✔️ development/2.10

The following branches have NOT changed:

development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

Please check the status of the associated issue None.

Goodbye teddyandrieux.

TeddyAndrieux requested a review from a team June 8, 2021 07:59

TeddyAndrieux force-pushed the improvement/refactore-cp-ingress-ep-definition branch from 9e57620 to bb8052a Compare June 8, 2021 08:00

TeddyAndrieux linked an issue Jun 8, 2021 that may be closed by this pull request

Control plane Ingress controller is tied to Bootstrap #2381

Closed

alexandre-allard reviewed Jun 8, 2021

View reviewed changes

NicolasT self-requested a review June 8, 2021 08:10

alexandre-allard reviewed Jun 9, 2021

View reviewed changes

TeddyAndrieux force-pushed the improvement/refactore-cp-ingress-ep-definition branch from bb8052a to 4eac18e Compare June 10, 2021 08:44

alexandre-allard previously approved these changes Jun 10, 2021

View reviewed changes

TeddyAndrieux mentioned this pull request Jun 11, 2021

Control Plane Ingress IP managed by MetalLB #3418

Merged

gdemonet suggested changes Jun 17, 2021

View reviewed changes

gdemonet mentioned this pull request Jun 18, 2021

tests: Validation of UI access does not handle custom base path #3421

Open

TeddyAndrieux added 5 commits June 21, 2021 15:21

salt: Define Control Plane Ingress IP and Endpoint at a single place

f86cb04

Instead of hardcode Control Plane Ingress Endpoint at several places in the salt states, add a new Salt module to "compute" it and use this salt module everywhere in our salt states

tests: Retrieve Control Plane Ingress Endpoint from Kubernetes Service

d410b52

Instead of hardcode Ingress port in tests and use Bootstrap Control Plane IP to compute the Control Plane Ingress Endpoint, retrieve it using the Kubernetes Service

docs: Update doc to reach the UI using Ingress CP endpoint

c6efd5a

Update the command to retrieve the IP using Kubernetes API and Control Plane Ingress Service to get the external IP

salt,docs: Make Ingress Control Plane IP configurable

967099a

Make the IP used to reach the UI and other Control Plane components configurable from Bootstrap config file Refs: #2381

tests: Make given running on multi node cluster more public

a5b85dd

Move `we are on a multi node cluster` from test registry to conftest, so that it can be used in other tests

TeddyAndrieux dismissed alexandre-allard’s stale review via d3e36a4 June 21, 2021 13:23

TeddyAndrieux force-pushed the improvement/refactore-cp-ingress-ep-definition branch from 4eac18e to d3e36a4 Compare June 21, 2021 13:23

gdemonet previously approved these changes Jun 23, 2021

View reviewed changes

docs/operation/changing_control_plane_ingress_ip.rst Outdated Show resolved Hide resolved

docs/operation/changing_control_plane_ingress_ip.rst Outdated Show resolved Hide resolved

salt,docs,tests: Add procedure to change the Control Plane Ingress IP

4fe7cac

Add a simple orchestrate and small procedure in the documentation to change the Control Plane Ingress IP

TeddyAndrieux dismissed gdemonet’s stale review via 4fe7cac June 23, 2021 09:16

TeddyAndrieux force-pushed the improvement/refactore-cp-ingress-ep-definition branch from d3e36a4 to 4fe7cac Compare June 23, 2021 09:16

gdemonet approved these changes Jun 23, 2021

View reviewed changes

bert-e merged commit 5806039 into development/2.10 Jun 23, 2021

bert-e deleted the improvement/refactore-cp-ingress-ep-definition branch June 23, 2021 12:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Control Plane Ingress IP configurable #3415

Make Control Plane Ingress IP configurable #3415

TeddyAndrieux commented Jun 8, 2021

bert-e commented Jun 8, 2021

bert-e commented Jun 8, 2021

alexandre-allard Jun 8, 2021

gdemonet left a comment

gdemonet Jun 17, 2021

TeddyAndrieux Jun 17, 2021 •

edited

Loading

gdemonet Jun 18, 2021

TeddyAndrieux Jun 18, 2021

gdemonet Jun 17, 2021

TeddyAndrieux Jun 17, 2021

gdemonet Jun 17, 2021

TeddyAndrieux Jun 17, 2021

gdemonet Jun 18, 2021

bert-e commented Jun 17, 2021

gdemonet left a comment

bert-e commented Jun 23, 2021

TeddyAndrieux commented Jun 23, 2021

bert-e commented Jun 23, 2021

bert-e commented Jun 23, 2021

Make Control Plane Ingress IP configurable #3415

Make Control Plane Ingress IP configurable #3415

Conversation

TeddyAndrieux commented Jun 8, 2021

bert-e commented Jun 8, 2021

Hello teddyandrieux,

bert-e commented Jun 8, 2021

Waiting for approval

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TeddyAndrieux Jun 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bert-e commented Jun 17, 2021

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Jun 23, 2021

Waiting for approval

TeddyAndrieux commented Jun 23, 2021

bert-e commented Jun 23, 2021

In the queue

bert-e commented Jun 23, 2021

TeddyAndrieux Jun 17, 2021 •

edited

Loading