Receive the API server endpoint URLs via kube-control #84

johnsca · 2021-04-06T20:43:05Z

This simplifies how the workers receive the API endpoint URLs by making them always come from the master via the kube-control relation. It also improves how status is reported when the endpoint address is not available.

Part of lp:1921776
Depends on juju-solutions/interface-kube-control#33

This adds support for the `loadbalancer` interface so that cloud-native LBs can be provided by the integrator charms. Additionally, it simplifies the confusing way the relations between the masters and workers change depending on whether kubeapi-load-balancer is being used or not by making that use the same `lb-provider` endpoint and always forwarding the API endpoint URLs via the `kube-control` relation. Part of [lp:1897818][] Depends on: * juju-solutions/loadbalancer-interface#13 * juju-solutions/interface-kube-control#33 * charmed-kubernetes/charm-kubernetes-worker#84 * charmed-kubernetes/charm-kubeapi-load-balancer#11 [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818

Simplify the relations between the master, worker, and API LB charms in favor of the newer `loadbalancer` interface relation. Part of [lp:1897818][] Depends on: * juju-solutions/loadbalancer-interface#13 * juju-solutions/interface-kube-control#33 * charmed-kubernetes/charm-kubernetes-control-plane#153 * charmed-kubernetes/charm-kubernetes-worker#84 * charmed-kubernetes/charm-kubeapi-load-balancer#11 [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818

Cynerva

Looks good, but needs integration test coverage.

Cynerva · 2021-04-14T17:11:02Z

reactive/kubernetes_worker.py

+        ]
+    if hasattr(kube_control, "get_api_endpoints"):
+        return kube_control.get_api_endpoints()
+    return []


This is one of those "should never happen" cases, and if encountered, will lead to hook errors since callers assume that at least 1 valid endpoint is returned. I would think it would be more fitting to raise an exception here, or at the very least, log a warning.

Cynerva

Changes look good, but this still needs integration tests.

This adds support for the `loadbalancer` interface so that cloud-native LBs can be provided by the integrator charms. Additionally, it simplifies the confusing way the relations between the masters and workers change depending on whether kubeapi-load-balancer is being used or not by making that use the same `lb-provider` endpoint and always forwarding the API endpoint URLs via the `kube-control` relation. Part of [lp:1897818][] Depends on: * juju-solutions/loadbalancer-interface#13 * juju-solutions/interface-kube-control#33 * charmed-kubernetes/charm-kubernetes-worker#84 * charmed-kubernetes/charm-kubeapi-load-balancer#11 [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818

This simplifies how the workers receive the API endpoint URLs by making them always come from the master via the `kube-control` relation. It also improves how status is reported when the endpoint address is not available. Part of [lp:1897818][] [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818

…y requirements

johnsca · 2021-05-11T20:41:07Z

Here's a manual test run with this plus charmed-kubernetes/charm-kubernetes-control-plane#153, charmed-kubernetes/charm-kubernetes-control-plane#156, and charmed-kubernetes/layer-kubernetes-common#17, using a master pre-deployed into the model. Note that to get the test to work with the pre-deployed worker, you have to use the master branch of python-libjuju (for juju/python-libjuju#488) until the next version is released to PyPI:

tox -re integration --notest
.tox/integration/bin/pip uninstall juju
.tox/integration/bin/pip install --no-cache https://github.com/juju/python-libjuju/archive/refs/heads/master.zip#egg=juju

This adds support for the `loadbalancer` interface so that cloud-native LBs can be provided by the integrator charms. Additionally, it simplifies the confusing way the relations between the masters and workers change depending on whether kubeapi-load-balancer is being used or not by making that use the same `lb-provider` endpoint and always forwarding the API endpoint URLs via the `kube-control` relation. Part of [lp:1897818][] Depends on: * juju-solutions/loadbalancer-interface#13 * juju-solutions/interface-kube-control#33 * charmed-kubernetes/charm-kubernetes-worker#84 * charmed-kubernetes/charm-kubeapi-load-balancer#11 [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818

Cynerva

Some suggestions but LGTM

Cynerva · 2021-05-19T18:02:07Z

tests/data/bundle.yaml

+    channel: edge
+    num_units: 1
+    resources:
+      easyrsa: 5


I would remove resource revisions from the bundle. No point specifying them when we don't specify charm revisions.

Cynerva · 2021-05-19T18:02:35Z

tests/data/bundle.yaml

+    expose: true
+    num_units: 1
+    options:
+      channel: 1.20/stable


1.21/stable or 1.22/edge?

Cynerva · 2021-05-19T18:02:45Z

tests/data/bundle.yaml

+    expose: true
+    num_units: 1
+    options:
+      channel: 1.20/stable


1.21/stable or 1.22/edge?

Cynerva · 2021-05-19T18:07:55Z

tests/integration/test_kubernetes_worker_integration.py

+@pytest.mark.abort_on_fail
+async def test_build_and_deploy(ops_test):
+    cni_amd64 = ops_test.tmp_path / "cni-amd64.tgz"
+    urlretrieve(CNI_AMD64_URL, cni_amd64)


Not a fan of the hard-coded CNI resource. What if someone changes the build script? The test wouldn't cover those changes.

I would consider building the resource here, just like we build the charm.

Cynerva · 2021-05-19T18:11:26Z

tests/integration/test_kubernetes_worker_integration.py

+    )
+
+    await ops_test.model.wait_for_idle(wait_for_active=True, timeout=10 * 60)
+    _check_status_messages(ops_test)


Consider removing the kube-api-endpoint relation at the end of the test, so future tests aren't running with weird relations in play.

* Add support for loadbalancer interface This adds support for the `loadbalancer` interface so that cloud-native LBs can be provided by the integrator charms. Additionally, it simplifies the confusing way the relations between the masters and workers change depending on whether kubeapi-load-balancer is being used or not by making that use the same `lb-provider` endpoint and always forwarding the API endpoint URLs via the `kube-control` relation. Part of [lp:1897818][] Depends on: * juju-solutions/loadbalancer-interface#13 * juju-solutions/interface-kube-control#33 * charmed-kubernetes/charm-kubernetes-worker#84 * charmed-kubernetes/charm-kubeapi-load-balancer#11 [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818 * Fix lint error * Add test for status reporting of incomplete LB relation * Fix lint errors in test * Fix hook error when built with old version of kube-control interface * Split lb-provider endpoint into separate, more explicit ones for internal / external * Fix handling of internal LB response and missing endpoints * Move test to new LB relation pattern and use edge charms * Improve status reporting around the auth-webhook and skip trying to create secrets before apiserver is available * Create a ~/.kube/config for the ubuntu user * Fix NoneType error for old-style relation * Fix unit test * Revert drive-by related to auth setup status reporting; splitting to separate bug

* Support loadbalancer interface between master and API LB Simplify the relations between the master, worker, and API LB charms in favor of the newer `loadbalancer` interface relation. Part of [lp:1897818][] Depends on: * juju-solutions/loadbalancer-interface#13 * juju-solutions/interface-kube-control#33 * charmed-kubernetes/charm-kubernetes-control-plane#153 * charmed-kubernetes/charm-kubernetes-worker#84 * charmed-kubernetes/charm-kubeapi-load-balancer#11 [lp:1897818]: https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/1897818 * Split k8s-master:lb-provider relation into external / internal * Update core and converged bundle fragments

johnsca mentioned this pull request Apr 6, 2021

Add support for loadbalancer interface charmed-kubernetes/charm-kubernetes-control-plane#153

Merged

johnsca mentioned this pull request Apr 6, 2021

Support loadbalancer interface between master and API LB charmed-kubernetes/bundle#796

Merged

Cynerva suggested changes Apr 14, 2021

View reviewed changes

johnsca requested a review from Cynerva April 22, 2021 14:44

Cynerva suggested changes Apr 22, 2021

View reviewed changes

johnsca marked this pull request as draft May 7, 2021 21:27

johnsca added 5 commits May 10, 2021 16:46

Fix hook error when built with old version of kube-control interface

7511d85

Prefer older kube-api-endpoints to support case of weird reverse-prox…

4680f36

…y requirements

Handle edge case of cluster endpoint not being available

8ec3ee2

Add integration test

20ffa9d

johnsca force-pushed the johnsca/lp/1921776/support-loadbalancer-interface branch from d4c27ee to 20ffa9d Compare May 10, 2021 20:46

Remove Travis

821c90c

johnsca marked this pull request as ready for review May 11, 2021 20:41

Include constraints in manual deploy step of integration test

0ebdf5c

johnsca mentioned this pull request May 13, 2021

Add support for loadbalancer interface charmed-kubernetes/charm-kubeapi-load-balancer#11

Merged

Use actions-operator on integration tests for vSphere environment

1817c2d

Cynerva approved these changes May 19, 2021

View reviewed changes

Cynerva merged commit 2ad6fb6 into master May 19, 2021

Cynerva deleted the johnsca/lp/1921776/support-loadbalancer-interface branch May 19, 2021 18:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Receive the API server endpoint URLs via kube-control #84

Receive the API server endpoint URLs via kube-control #84

johnsca commented Apr 6, 2021 •

edited

Loading

Cynerva left a comment

Cynerva Apr 14, 2021

Cynerva left a comment

johnsca commented May 11, 2021

Cynerva left a comment

Cynerva May 19, 2021

Cynerva May 19, 2021

Cynerva May 19, 2021

Cynerva May 19, 2021

Cynerva May 19, 2021

Receive the API server endpoint URLs via kube-control #84

Receive the API server endpoint URLs via kube-control #84

Conversation

johnsca commented Apr 6, 2021 • edited Loading

Cynerva left a comment

Choose a reason for hiding this comment

Cynerva Apr 14, 2021

Choose a reason for hiding this comment

Cynerva left a comment

Choose a reason for hiding this comment

johnsca commented May 11, 2021

Cynerva left a comment

Choose a reason for hiding this comment

Cynerva May 19, 2021

Choose a reason for hiding this comment

Cynerva May 19, 2021

Choose a reason for hiding this comment

Cynerva May 19, 2021

Choose a reason for hiding this comment

Cynerva May 19, 2021

Choose a reason for hiding this comment

Cynerva May 19, 2021

Choose a reason for hiding this comment

johnsca commented Apr 6, 2021 •

edited

Loading