From 63f07887479f0f300dbf18b9ab1afe4b2aae6d7b Mon Sep 17 00:00:00 2001 From: Allison Larson Date: Mon, 30 Jun 2025 14:32:23 -0700 Subject: [PATCH] Expose Kind field for Consul Service Registrations (#26170) * consul: Add service kind to jobspec * consul: Add kind to service docs * Add changelog --- .changelog/26170.txt | 3 + api/services.go | 3 + command/agent/consul/service_client.go | 7 +- command/agent/consul/unit_test.go | 39 ++++++ command/agent/job_endpoint.go | 1 + command/agent/job_endpoint_test.go | 2 + nomad/structs/diff_test.go | 118 ++++++++++++++++++ nomad/structs/services.go | 22 ++++ nomad/structs/services_test.go | 26 ++++ .../docs/job-specification/service.mdx | 6 + 10 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 .changelog/26170.txt diff --git a/.changelog/26170.txt b/.changelog/26170.txt new file mode 100644 index 000000000..16816f0cc --- /dev/null +++ b/.changelog/26170.txt @@ -0,0 +1,3 @@ +```release-note:improvement +consul: Added kind field to service block for Consul service registrations +``` diff --git a/api/services.go b/api/services.go index eac749470..98c59b80f 100644 --- a/api/services.go +++ b/api/services.go @@ -254,6 +254,9 @@ type Service struct { // Cluster is valid only for Nomad Enterprise with provider: consul Cluster string `hcl:"cluster,optional"` + + // Kind defines the consul service kind, valid only when provider: consul + Kind string `hcl:"kind,optional"` } const ( diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go index 6589a6d41..762a9a2ff 100644 --- a/command/agent/consul/service_client.go +++ b/command/agent/consul/service_client.go @@ -1362,9 +1362,10 @@ func (c *ServiceClient) serviceRegs( // This enables the consul UI to show that Nomad registered this service meta["external-source"] = "nomad" - // Explicitly set the Consul service Kind in case this service represents - // one of the Connect gateway types. - kind := api.ServiceKindTypical + // Set the Consul service Kind from the service.Kind, to be overwritten if + // the service is connect gateway (empty string is api.ServiceKindTypical) + kind := api.ServiceKind(service.Kind) + switch { case service.Connect.IsIngress(): kind = api.ServiceKindIngressGateway diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index 123770f58..8364029d3 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -1050,6 +1050,45 @@ func TestConsul_ServiceName_Duplicates(t *testing.T) { } } +func TestConsul_ServiceKind(t *testing.T) { + ci.Parallel(t) + ctx := setupFake(t) + + ctx.Workload.Services = []*structs.Service{ + { + Name: "best-service", + PortLabel: "x", + Kind: "api-gateway", + }, + { + Name: "service", + PortLabel: "y", + }, + { + Name: "worst-service", + PortLabel: "y", + Kind: "terminating-gateway", + }, + } + + must.NoError(t, ctx.ServiceClient.RegisterWorkload(ctx.Workload)) + must.NoError(t, ctx.syncOnce(syncNewOps)) + must.MapLen(t, 3, ctx.FakeConsul.services["default"]) + + for _, s := range ctx.FakeConsul.services["default"] { + switch { + case s.Name == "best-service": + must.Eq(t, "api-gateway", s.Kind) + case s.Name == "service" && s.Port == yPort: + must.Eq(t, "", s.Kind) + case s.Name == "worst-service": + must.Eq(t, "terminating-gateway", s.Kind) + default: + t.Fatalf("unexpected service: %s", s.Name) + } + } +} + // TestConsul_ServiceDeregistration_OutOfProbation asserts that during in steady // state we remove any services we don't reconize locally func TestConsul_ServiceDeregistration_OutProbation(t *testing.T) { diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 693317c0d..a6de14c43 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1713,6 +1713,7 @@ func ApiServicesToStructs(in []*api.Service, group bool) []*structs.Service { OnUpdate: s.OnUpdate, Provider: s.Provider, Cluster: s.Cluster, + Kind: s.Kind, } if l := len(s.Checks); l != 0 { diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 611568abd..0daf128af 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2890,6 +2890,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { FailuresBeforeWarning: 2, }, }, + Kind: "api-gateway", Connect: &api.ConsulConnect{ Native: false, SidecarService: &api.ConsulSidecarService{ @@ -3302,6 +3303,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Passing: 5, Warning: 1, }, + Kind: "api-gateway", OnUpdate: structs.OnUpdateRequireHealthy, Checks: []*structs.ServiceCheck{ { diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 8dc77084e..fb845f96a 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -3586,6 +3586,12 @@ func TestTaskGroupDiff(t *testing.T) { Old: "false", New: "true", }, + { + Type: DiffTypeNone, + Name: "Kind", + Old: "", + New: "", + }, { Type: DiffTypeNone, Name: "Name", @@ -7078,6 +7084,10 @@ func TestTaskDiff(t *testing.T) { Old: "false", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeNone, Name: "Name", @@ -7244,6 +7254,10 @@ func TestTaskDiff(t *testing.T) { Old: "false", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeNone, Name: "Name", @@ -7794,6 +7808,10 @@ func TestTaskDiff(t *testing.T) { Old: "false", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeNone, Name: "Name", @@ -9558,6 +9576,10 @@ func TestServicesDiff(t *testing.T) { Old: "true", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeEdited, Name: "Name", @@ -9677,6 +9699,10 @@ func TestServicesDiff(t *testing.T) { Name: "EnableTagOverride", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeAdded, Name: "Name", @@ -9750,6 +9776,10 @@ func TestServicesDiff(t *testing.T) { Name: "EnableTagOverride", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeAdded, Name: "Name", @@ -9828,6 +9858,10 @@ func TestServicesDiff(t *testing.T) { Old: "false", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeNone, Name: "Name", @@ -9859,6 +9893,82 @@ func TestServicesDiff(t *testing.T) { }, }, }, + { + Name: "Modify service kind field", + Contextual: true, + Old: []*Service{ + { + Name: "webapp", + Kind: "api-gateway", + }, + }, + New: []*Service{ + { + Name: "webapp", + Kind: "mesh-gateway", + }, + }, + Expected: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Service", + Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Address", + }, + { + Type: DiffTypeNone, + Name: "AddressMode", + }, + { + Type: DiffTypeNone, + Name: "Cluster", + Old: "", + New: "", + }, + { + Type: DiffTypeNone, + Name: "EnableTagOverride", + Old: "false", + New: "false", + }, + { + Type: DiffTypeEdited, + Name: "Kind", + Old: "api-gateway", + New: "mesh-gateway", + }, + { + Type: DiffTypeNone, + Name: "Name", + Old: "webapp", + New: "webapp", + }, + { + Type: DiffTypeNone, + Name: "Namespace", + }, + { + Type: DiffTypeNone, + Name: "OnUpdate", + }, + { + Type: DiffTypeNone, + Name: "PortLabel", + }, + { + Type: DiffTypeNone, + Name: "Provider", + }, + { + Type: DiffTypeNone, + Name: "TaskName", + }, + }, + }, + }, + }, { Name: "Modify similar services", Contextual: true, @@ -9911,6 +10021,10 @@ func TestServicesDiff(t *testing.T) { Old: "false", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeNone, Name: "Name", @@ -10006,6 +10120,10 @@ func TestServicesDiff(t *testing.T) { Old: "false", New: "false", }, + { + Type: DiffTypeNone, + Name: "Kind", + }, { Type: DiffTypeNone, Name: "Name", diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 09acf06bd..2ab5d4561 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -646,6 +646,10 @@ type Service struct { // Its name will be `consul-service/${service_name}`, and its contents will // match the server's `consul.service_identity` configuration block. Identity *WorkloadIdentity + + // Kind defines the Consul service kind, valid only when provider is consul + // and a Consul Connect Gateway isn't defined for the service. + Kind string } // Copy the block recursively. Returns nil if nil. @@ -675,6 +679,8 @@ func (s *Service) Copy() *Service { ns.Weights = s.Weights.Copy() ns.Identity = s.Identity.Copy() + ns.Kind = s.Kind + return ns } @@ -861,6 +867,17 @@ func (s *Service) validateConsulService(mErr *multierror.Error) { } } + // validate the consul service kind + switch api.ServiceKind(s.Kind) { + case api.ServiceKindTypical, + api.ServiceKindAPIGateway, + api.ServiceKindIngressGateway, + api.ServiceKindMeshGateway, + api.ServiceKindTerminatingGateway: + default: + mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s kind must be one of consul service kind or empty", s.Name)) + } + // check connect if s.Connect != nil { if err := s.Connect.Validate(); err != nil { @@ -957,6 +974,7 @@ func (s *Service) Hash(allocID, taskName string, canary bool) string { hashString(h, s.Namespace) hashIdentity(h, s.Identity) hashWeights(h, s.Weights) + hashString(h, s.Kind) // Don't hash the provider parameter, so we don't cause churn of all // registered services when upgrading Nomad versions. The provider is not @@ -1137,6 +1155,10 @@ func (s *Service) Equal(o *Service) bool { return false } + if s.Kind != o.Kind { + return false + } + return true } diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index add2cb1aa..731369b96 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -492,6 +492,10 @@ func TestService_Hash(t *testing.T) { try(t, func(s *svc) { s.PortLabel = "newPortLabel" }) }) + t.Run("mod kind", func(t *testing.T) { + try(t, func(s *svc) { s.Kind = "api-gateway" }) + }) + t.Run("mod tags", func(t *testing.T) { try(t, func(s *svc) { s.Tags = []string{"new", "tags"} }) }) @@ -2039,6 +2043,25 @@ func TestService_Validate(t *testing.T) { expErr: true, expErrStr: "notes must not be longer than 255 characters", }, + { + name: "provider consul with service kind", + input: &Service{ + Name: "testservice", + Provider: "consul", + Kind: "api-gateway", + }, + expErr: false, + }, + { + name: "provider consul with invalid service kind", + input: &Service{ + Name: "testservice", + Provider: "consul", + Kind: "garbage", + }, + expErr: true, + expErrStr: "Service testservice kind must be one of consul service kind or empty", + }, } for _, tc := range testCases { @@ -2139,6 +2162,9 @@ func TestService_Equal(t *testing.T) { o.TaggedAddresses = map[string]string{"foo": "bar"} assertDiff() + + o.Kind = "api-gateway" + assertDiff() } func TestService_validateNomadService(t *testing.T) { diff --git a/website/content/docs/job-specification/service.mdx b/website/content/docs/job-specification/service.mdx index bfed648bd..c03756a33 100644 --- a/website/content/docs/job-specification/service.mdx +++ b/website/content/docs/job-specification/service.mdx @@ -108,6 +108,11 @@ Service Mesh][connect] integration. - `connect` - Configures the [Consul Connect][connect] integration. Only available on group services and where `provider = "consul"`. +- `kind` `(string: )` - Configures the [Consul Service +Kind][consul_kind] to pass to Consul during service registration. Only available +when `provider = "consul"`, and is ignored if a Consul Connect Gateway is +defined. + - `identity` ([Identity][identity_block]: nil) - Specifies a Workload Identity to use when obtaining Service Identity tokens from Consul to register the service. Only available where `provider = "consul"`. Typically @@ -537,6 +542,7 @@ advertise and check directly since Nomad isn't managing any port assignments. [qemu]: /nomad/docs/drivers/qemu 'Nomad QEMU Driver' [restart_block]: /nomad/docs/job-specification/restart 'restart block' [connect]: /nomad/docs/job-specification/connect 'Nomad Consul Connect Integration' +[kind]: /consul/api-docs/agent/service#kind [type]: /nomad/docs/job-specification/service#type [shutdowndelay]: /nomad/docs/job-specification/task#shutdown_delay [killsignal]: /nomad/docs/job-specification/task#kill_signal