Skip to content

Commit

Permalink
Was missing namespace and partition
Browse files Browse the repository at this point in the history
  • Loading branch information
curtbushko committed May 16, 2023
1 parent a74d327 commit c82e4c5
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,8 @@ func assignServiceVirtualIP(ctx context.Context, apiClient *api.Client, svc *api
if ip == "" {
return nil
}
_, _, err := apiClient.Internal().AssignServiceVirtualIP(ctx, svc.Service, []string{ip}, &api.WriteOptions{})

_, _, err := apiClient.Internal().AssignServiceVirtualIP(ctx, svc.Service, []string{ip}, &api.WriteOptions{Namespace: svc.Namespace, Partition: svc.Partition})
if err != nil {
// Maintain backwards compatibility with older versions of Consul that do not support the VIP improvements. Tproxy
// will not work 100% correctly but the mesh will still work
Expand Down
11 changes: 8 additions & 3 deletions control-plane/controllers/configentry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (r *ConfigEntryController) ReconcileEntry(ctx context.Context, crdCtrl Cont
// For resolvers and splitters, we need to set the ClusterIP of the matching service to Consul so that transparent
// proxy works correctly. Do not fail the reconcile if assigning the virtual IP returns an error.
if needsVirtualIPAssignment(configEntry) {
err = assignServiceVirtualIP(ctx, logger, consulClient, crdCtrl, req.NamespacedName, configEntry)
err = assignServiceVirtualIP(ctx, logger, consulClient, crdCtrl, req.NamespacedName, configEntry, r.DatacenterName)
if err != nil {
logger.Error(err, "failed assigning service virtual ip")
}
Expand Down Expand Up @@ -405,7 +405,7 @@ func needsVirtualIPAssignment(configEntry common.ConfigEntryResource) bool {
// does not exist or if an older version of Consul is being used. Endpoints Controller, on service registration, also
// manually sends a ClusterIP when a service is created. This increases the chance of a real IP ending up in the
// discovery chain.
func assignServiceVirtualIP(ctx context.Context, logger logr.Logger, consulClient *capi.Client, crdCtrl Controller, namespacedName types.NamespacedName, configEntry common.ConfigEntryResource) error {
func assignServiceVirtualIP(ctx context.Context, logger logr.Logger, consulClient *capi.Client, crdCtrl Controller, namespacedName types.NamespacedName, configEntry common.ConfigEntryResource, datacenter string) error {
service := corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: configEntry.KubernetesName(),
Expand All @@ -422,8 +422,13 @@ func assignServiceVirtualIP(ctx context.Context, logger logr.Logger, consulClien
return err
}

wo := &capi.WriteOptions{
Namespace: configEntry.ToConsul(datacenter).GetNamespace(),
Partition: configEntry.ToConsul(datacenter).GetPartition(),
}

logger.Info("adding manual ip to virtual ip table in Consul", "name", service.Name)
_, _, err := consulClient.Internal().AssignServiceVirtualIP(ctx, configEntry.KubernetesName(), []string{service.Spec.ClusterIP}, &capi.WriteOptions{})
_, _, err := consulClient.Internal().AssignServiceVirtualIP(ctx, configEntry.KubernetesName(), []string{service.Spec.ClusterIP}, wo)
if err != nil {
// Maintain backwards compatibility with older versions of Consul that do not support the manual VIP improvements. With the older version, the mesh
// will still work.
Expand Down
2 changes: 1 addition & 1 deletion control-plane/controllers/configentry_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2115,7 +2115,7 @@ func TestConfigEntryControllers_assignServiceVirtualIP(t *testing.T) {
Name: c.configEntryResource.KubernetesName(),
}

err := assignServiceVirtualIP(ctx, ctrl.Logger(namespacedName), consulClient, ctrl, namespacedName, c.configEntryResource)
err := assignServiceVirtualIP(ctx, ctrl.Logger(namespacedName), consulClient, ctrl, namespacedName, c.configEntryResource, "dc1")
if err != nil {
require.True(t, c.expectErr)
} else {
Expand Down

0 comments on commit c82e4c5

Please sign in to comment.