From 646da639eed1cb20d10d2c399cb13ee155a7f85a Mon Sep 17 00:00:00 2001 From: Chris Piraino Date: Fri, 15 Nov 2019 09:06:33 -0600 Subject: [PATCH] Allow services register command to register an unnamed check The logic in parsing data files and converting them to data structures accidentally removed healthchecks with no Name field, even though we explicitly state in API documentation that is allowed. We remove the check for "len(results.Checks) == 1" because if the length of the array is more than 0, we know that it is not a zero value array. This allows us to register a singular, unnamed check via the CLI. Fixes #6796 --- command/services/config.go | 7 ++-- command/services/config_test.go | 27 ++++++++++++++++ command/services/register/register_test.go | 37 ++++++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/command/services/config.go b/command/services/config.go index 2241c90eca..ee2477d3c4 100644 --- a/command/services/config.go +++ b/command/services/config.go @@ -65,13 +65,10 @@ func serviceToAgentService(svc *structs.ServiceDefinition) (*api.AgentServiceReg // The structs version has non-pointer checks and the destination // has pointers, so we need to set the destination to nil if there - // is no check ID set. - if result.Check != nil && result.Check.Name == "" { + // is a zero-value Check field. + if result.Check != nil && reflect.DeepEqual(*result.Check, api.AgentServiceCheck{}) { result.Check = nil } - if len(result.Checks) == 1 && result.Checks[0].Name == "" { - result.Checks = nil - } return &result, nil } diff --git a/command/services/config_test.go b/command/services/config_test.go index d98e52a19f..d6e93b3dba 100644 --- a/command/services/config_test.go +++ b/command/services/config_test.go @@ -2,6 +2,7 @@ package services import ( "testing" + "time" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/structs" @@ -63,6 +64,32 @@ func TestStructsToAgentService(t *testing.T) { }, }, }, + { + "Service with an unnamed check", + &structs.ServiceDefinition{ + Name: "web", + Check: structs.CheckType{ + TTL: 5 * time.Second, + }, + }, + &api.AgentServiceRegistration{ + Name: "web", + Check: &api.AgentServiceCheck{ + TTL: "5s", + }, + }, + }, + { + "Service with a zero-value check", + &structs.ServiceDefinition{ + Name: "web", + Check: structs.CheckType{}, + }, + &api.AgentServiceRegistration{ + Name: "web", + Check: nil, + }, + }, { "Service with checks", &structs.ServiceDefinition{ diff --git a/command/services/register/register_test.go b/command/services/register/register_test.go index fa2d95daae..e00e736267 100644 --- a/command/services/register/register_test.go +++ b/command/services/register/register_test.go @@ -153,6 +153,43 @@ func TestCommand_Flags_TaggedAddresses(t *testing.T) { require.Equal(svc.TaggedAddresses["v6"].Port, 1234) } +func TestCommand_FileWithUnnamedCheck(t *testing.T) { + t.Parallel() + + require := require.New(t) + a := agent.NewTestAgent(t, t.Name(), ``) + defer a.Shutdown() + client := a.Client() + + ui := cli.NewMockUi() + c := New(ui) + + contents := `{ "Service": { "Name": "web", "Check": { "TTL": "10s" } } }` + f := testFile(t, "json") + defer os.Remove(f.Name()) + if _, err := f.WriteString(contents); err != nil { + t.Fatalf("err: %#v", err) + } + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + f.Name(), + } + + require.Equal(0, c.Run(args), ui.ErrorWriter.String()) + + svcs, err := client.Agent().Services() + require.NoError(err) + require.Len(svcs, 1) + + svc := svcs["web"] + require.NotNil(svc) + + checks, err := client.Agent().Checks() + require.NoError(err) + require.Len(checks, 1) +} + func testFile(t *testing.T, suffix string) *os.File { f := testutil.TempFile(t, "register-test-file") if err := f.Close(); err != nil {