operator/upgrade: switch from kafka to admin API for redpanda readiness #1159

dimitriscruz · 2021-04-14T06:30:42Z

Cover letter

With the introduction of SASL, calling any Kafka listener requires credentials. Although that is possible to do from the operator, it adds complexity.
It is our intention to use an upgrade-specific endpoint, which would be part of the http Admin API (not the Kafka API)

Due to the above, this PR changes the operator behavior such that it check the redpanda readiness using the Admin API. Since we don't have the ideal endpoint at the moment, we start by using v1/state/ready with the intention to develop an appropriate endpoint in the (hopefully near) future.

alenkacz

can't we use readiness probe for this? The advantage of that would be that the pod would be properly in API marked as non-ready until ready which has implication for all kind of stuff like registering services etc. Also it's transparent for anyone using the kubernetes API...

dimitriscruz · 2021-04-14T15:24:27Z

can't we use readiness probe for this? The advantage of that would be that the pod would be properly in API marked as non-ready until ready which has implication for all kind of stuff like registering services etc. Also it's transparent for anyone using the kubernetes API...

Given that this PR uses v1/state/ready, we might be able to use a readiness probe. However, the point of this PR is to prepare for an upgrade-specific endpoint, which wouldn't apply as a generic one.

alenkacz

Looks good, can you run go mod tidy though? I think the sarama dep should go away

alenkacz · 2021-04-14T15:46:58Z

src/go/k8s/controllers/redpanda/cluster_controller.go

@@ -112,6 +112,7 @@ func (r *ClusterReconciler) Reconcile(
 		pki.OperatorClientCert(),


this is getting really bad, I need to take a look at this

Yeah, maybe we could group them together in structs like "pkiInfo"

Similarly for the services

I did go mod tidy also. No sarama dep anymore.

src/go/k8s/tests/e2e/update-image/00-current-image.yaml

emaxerrno · 2021-04-14T20:07:53Z

src/go/k8s/pkg/resources/statefulset_update.go


 	// TODO right now we support TLS only on one listener so if external
 	// connectivity is enabled, TLS is enabled only on external listener. This
 	// will be fixed by https://github.com/vectorizedio/redpanda/issues/1084
-	if !r.pandaCluster.Spec.ExternalConnectivity.Enabled && r.pandaCluster.Spec.Configuration.TLS.KafkaAPI.Enabled {
+	if !r.pandaCluster.Spec.ExternalConnectivity.Enabled && r.pandaCluster.Spec.Configuration.TLS.AdminAPI.Enabled {


now that we have admin api with multiple interfaces, should we update this PR too ?

maybe choose the internal IP (but keep the tls check) so that at least the traffic doesn't go out through the internet.

We have an external listener but do not expose it in the API yet. This check is based on the API fields. So, we have the following convention: if TLS is enabled we apply it to the external listener. If there is no external listener (external connectivity is disabled), we apply the TLS to the internal listener.

Once #1163 is merged, we'll update this line and be explicit about the listener.

But just to be clear, we are using the internal listener here.

makes sense.

…atus

…-update operator/upgrade: switch from kafka to admin API for redpanda readiness

…ge-update operator/upgrade: switch from kafka to admin API for redpanda readiness

github-actions bot added the area/k8s label Apr 14, 2021

dimitriscruz requested review from RafalKorepta and alenkacz April 14, 2021 06:32

alenkacz reviewed Apr 14, 2021

View reviewed changes

alenkacz previously approved these changes Apr 14, 2021

View reviewed changes

dimitriscruz dismissed alenkacz’s stale review via 21b5bc1 April 14, 2021 16:13

emaxerrno reviewed Apr 14, 2021

View reviewed changes

src/go/k8s/tests/e2e/update-image/00-current-image.yaml Outdated Show resolved Hide resolved

emaxerrno reviewed Apr 14, 2021

View reviewed changes

dimitriscruz force-pushed the image-update branch from 21b5bc1 to 99da3d0 Compare April 15, 2021 04:45

dimitriscruz added 5 commits April 14, 2021 21:45

operator/pki: pass admin api client certificate key

3d651d3

operator/upgrade: only use client keys when client auth enabled

f5a7a7d

operator/upgrade: append err string in requeue msg

03e1e7e

operator/upgrade: replace kafka with admin api to check for broker st…

9f6d810

…atus

operator/upgrade: bring back broker authentication on client

69751ee

dimitriscruz force-pushed the image-update branch from 99da3d0 to 7f06022 Compare April 15, 2021 04:50

dimitriscruz marked this pull request as ready for review April 15, 2021 05:26

dimitriscruz requested a review from a team as a code owner April 15, 2021 05:26

RafalKorepta previously approved these changes Apr 15, 2021

View reviewed changes

dimitriscruz added 2 commits April 15, 2021 09:22

operator/upgrade: replace kafka with admin api in upgrade tests

e9d43c7

operator: remove sarama dependency

1bed80f

dimitriscruz dismissed RafalKorepta’s stale review via 1bed80f April 15, 2021 16:23

dimitriscruz force-pushed the image-update branch from 7f06022 to 1bed80f Compare April 15, 2021 16:23

RafalKorepta approved these changes Apr 15, 2021

View reviewed changes

emaxerrno merged commit d6f57e2 into redpanda-data:dev Apr 15, 2021

dimitriscruz deleted the image-update branch April 16, 2021 00:12

snyk-bot mentioned this pull request Aug 6, 2022

[Snyk] Upgrade chokidar from 3.4.3 to 3.5.3 runsafesecurity/redpanda#5

Open

wreet mentioned this pull request Oct 6, 2022

[Snyk] Upgrade chokidar from 3.4.3 to 3.5.3 #6655

Closed

joejulian pushed a commit to joejulian/redpanda that referenced this pull request Mar 10, 2023

(split) Merge pull request redpanda-data#1159 from dimitriscruz/image…

2855eb9

…-update operator/upgrade: switch from kafka to admin API for redpanda readiness

joejulian pushed a commit to joejulian/redpanda that referenced this pull request Mar 24, 2023

operator: Merge pull request redpanda-data#1159 from dimitriscruz/ima…

a76441a

…ge-update operator/upgrade: switch from kafka to admin API for redpanda readiness

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

operator/upgrade: switch from kafka to admin API for redpanda readiness #1159

operator/upgrade: switch from kafka to admin API for redpanda readiness #1159

dimitriscruz commented Apr 14, 2021 •

edited

Loading

alenkacz left a comment

dimitriscruz commented Apr 14, 2021

alenkacz left a comment

alenkacz Apr 14, 2021

dimitriscruz Apr 14, 2021 •

edited

Loading

dimitriscruz Apr 14, 2021

dimitriscruz Apr 15, 2021

emaxerrno Apr 14, 2021

dimitriscruz Apr 15, 2021

dimitriscruz Apr 15, 2021 •

edited

Loading

emaxerrno Apr 15, 2021

		@@ -112,6 +112,7 @@ func (r *ClusterReconciler) Reconcile(
		pki.OperatorClientCert(),

operator/upgrade: switch from kafka to admin API for redpanda readiness #1159

operator/upgrade: switch from kafka to admin API for redpanda readiness #1159

Conversation

dimitriscruz commented Apr 14, 2021 • edited Loading

Cover letter

alenkacz left a comment

Choose a reason for hiding this comment

dimitriscruz commented Apr 14, 2021

alenkacz left a comment

Choose a reason for hiding this comment

alenkacz Apr 14, 2021

Choose a reason for hiding this comment

dimitriscruz Apr 14, 2021 • edited Loading

Choose a reason for hiding this comment

dimitriscruz Apr 14, 2021

Choose a reason for hiding this comment

dimitriscruz Apr 15, 2021

Choose a reason for hiding this comment

emaxerrno Apr 14, 2021

Choose a reason for hiding this comment

dimitriscruz Apr 15, 2021

Choose a reason for hiding this comment

dimitriscruz Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

emaxerrno Apr 15, 2021

Choose a reason for hiding this comment

dimitriscruz commented Apr 14, 2021 •

edited

Loading

dimitriscruz Apr 14, 2021 •

edited

Loading

dimitriscruz Apr 15, 2021 •

edited

Loading