Add replace-existing-checks param to the api package #6496

[fouadz]

Add replace-existing-checks param to the API package #6496

[hashicorp-cla]

All committers have signed the CLA.

[hanshasselberg]

Thank you for your work @fouadz! There is another PR for feature #6541, which has a flag in ServiceRegistration which I would prefer to have instead of another function. But yours definitely has the better test coverage! :)

What do you think about the flag instead of the function?

Thanks!

[fouadz]

Hi @i0rek

This is my view based on very limited knowledge of the codebase.

The AgentServiceRegistration struct perfectly matches what you expect from the API payload.
One can be confused in thinking the API now supports replace_existing_checks in the payload.

Example:

{
  "service":  {
   "name": "web",
   "tags": ["rails"],
   "port": 80,
   "replace_existing_checks": true
  }
  ....
}

Also, we have to ensure that we support the new field everywhere else in the code.
We may also have to ignore or hide the field from results. I assume that's why @Aestek added the "-"

ReplaceExistingChecks bool json:"-"

However, I don't mind changing to a flag instead, your call.

Thanks!

[hanshasselberg]

Thats a good point! After checking what we did before, I suggest you add an new function that can be extended in future: ServiceRegisterOpts similar to

consul/api/agent.go

Lines 512 to 552 in 61d8778

	// Members returns the known gossip members. The WAN
	// flag can be used to query a server for WAN members.
	func (a *Agent) Members(wan bool) ([]*AgentMember, error) {
	r := a.c.newRequest("GET", "/v1/agent/members")
	if wan {
	r.params.Set("wan", "1")
	}
	_, resp, err := requireOK(a.c.doRequest(r))
	if err != nil {
	return nil, err
	}
	defer resp.Body.Close()
	var out []*AgentMember
	if err := decodeBody(resp, &out); err != nil {
	return nil, err
	}
	return out, nil
	}
	// MembersOpts returns the known gossip members and can be passed
	// additional options for WAN/segment filtering.
	func (a *Agent) MembersOpts(opts MembersOpts) ([]*AgentMember, error) {
	r := a.c.newRequest("GET", "/v1/agent/members")
	r.params.Set("segment", opts.Segment)
	if opts.WAN {
	r.params.Set("wan", "1")
	}
	_, resp, err := requireOK(a.c.doRequest(r))
	if err != nil {
	return nil, err
	}
	defer resp.Body.Close()
	var out []*AgentMember
	if err := decodeBody(resp, &out); err != nil {
	return nil, err
	}
	return out, nil
	}

.

What do you think?

[fouadz]

I like the idea, I will check how to implement the suggested fix tonight.
Thank you @i0rek

[hanshasselberg]

Looks great! Thanks for your work and time to get this done!