-
Notifications
You must be signed in to change notification settings - Fork 326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NET-6938] Create workloads in Consul for mesh gateway pods #3382
Conversation
4a4bee7
to
a0675b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal review
@@ -336,9 +336,11 @@ func (r *Controller) writeWorkload(ctx context.Context, pod corev1.Pod) error { | |||
} | |||
data := inject.ToProtoAny(workload) | |||
|
|||
resourceID := getWorkloadID(pod.GetName(), r.getConsulNamespace(pod.Namespace), r.getPartition()) | |||
r.Log.Info("registering workload with Consul", getLogFieldsForResource(resourceID)...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds helpful logging analogous to what we do elsewhere
consul-k8s/control-plane/connect-inject/controllers/serviceaccount/serviceaccount_controller.go
Line 112 in 2b0ef6a
r.Log.Info("registering workload identity with Consul", getLogFieldsForResource(workloadIdentityResource.Id)...) |
consul-k8s/control-plane/connect-inject/controllers/endpointsv2/endpoints_controller.go
Line 336 in 2b0ef6a
r.Log.Info("writing service to Consul", getLogFieldsForResource(consulSvcResource.Id)...) |
if pod.Annotations == nil { | ||
return false | ||
} | ||
if anno, ok := pod.Annotations[constants.AnnotationGatewayKind]; ok && anno != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This utilizes a known annotation added to all v2 mesh gateway (and api/terminating in the future) pods
constants.AnnotationGatewayKind: meshGatewayAnnotationKind, |
@@ -762,3 +764,11 @@ func getDestinationsID(name, namespace, partition string) *pbresource.ID { | |||
}, | |||
} | |||
} | |||
|
|||
func getLogFieldsForResource(id *pbresource.ID) []any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be helpful to move this out into a commonly-used package at some point but didn't do this now
Gateway pods are not mesh-injected because it doesn't make sense for an Envoy proxy workload to have a sidecar; however, they still need workloads created in Consul for them.
e89f00e
to
d4cd4ee
Compare
d4cd4ee
to
e518860
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks for walking me through it
* Create workload in Consul for gateway pods Gateway pods are not mesh-injected because it doesn't make sense for an Envoy proxy workload to have a sidecar; however, they still need workloads created in Consul for them. * Log when pod controller creates workload in Consul * Disable t-proxy probe overwrite for mesh gateway pods * Update test assertions * Add test case for gateway pod reconciliation
Changes proposed in this PR
The
Pod
controller currently only createsWorkloads
in Consul for mesh-injectedPods
.Pods
for xGateways are not mesh-injected because it doesn't make sense to add a proxy sidecar to aPod
that's already running a proxy as its primary workload; however, they still requireWorkloads
to be registered in Consul so that we can generateProxyStateTemplates
for them (in a future PR).This PR modifies the
Pod
controller to createWorkloads
in Consul for xGateway pods as well, relying on the fact that a known annotation specifying thegateway-kind
is added to all v2 xGatewayPods
.Notably, this also results in a
ProxyConfiguration
being created for the mesh gateway workload, and I disabled the health endpoint override functionality by adding a predefined annotation to the gateway pods as it doesn't make sense for workloads that are not mesh-injected/don't have a sidecar.How I've tested this PR
helm install
frommain
with this build forglobal.imageK8S
,meshGateway.enabled=true
, andexperiments=[resource-apis]
.Verify that the
Workload
andProxyConfiguration
are created in Consul by shelling into consul-server and using the following commands:Example commands + output
Workload
resource:ProxyConfiguration
resource:How I expect reviewers to test this PR
See above
Checklist