Skip to content
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.

Sign up for GitHub

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

api: enable query options on agent force-leave endpoint #15987

Merged
merged 3 commits into from
Apr 18, 2023
Merged

api: enable query options on agent force-leave endpoint #15987

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4adb08b
api: enable query options on agent force-leave endpoint
andreikom Mar 27, 2023
19f5848
api: enable query options on agent force-leave endpoint - unit test
andreikom Apr 18, 2023
63cbdda
deprecate ForceLeaveOpts
andreikom Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/15987.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
api: Enable setting query options on agent force-leave endpoint.
```
7 changes: 7 additions & 0 deletions api/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,14 @@ func (a *Agent) ForceLeavePrune(node string) error {
// ForceLeaveOpts is used to have the agent eject a failed node or remove it
// completely from the list of members.
func (a *Agent) ForceLeaveOpts(node string, opts ForceLeaveOpts) error {
Copy link
Collaborator

@huikang huikang Apr 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, do we still need ForceLeaveOpts? Also could you

  • add a test case to agent_test.go
  • update the api doc

Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@huikang : Is there anything to update in the API docs for this endpoint? It seems like this PR is meant to correct intended behavior (to be consistent with the docs), rather than to change behavior / deviate from the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a test
I didn't want to remove ForceLeaveOpts to avoid breaking anyone currently using it.
The Consul API doesn't change, and it already supports reading and utilizing an ACL token. It's just the client side that didn't have this option.

Let me know if something else is needed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkirschner-hashicorp , you are right. No change is needed for API doc.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could mark ForceLeaveOpts deprecated.

// ForceLeaveOpts is used to have the agent eject a failed node or remove it
// completely from the list of members.
// 
// DEPRECATED - Use ForceLeaveOptions instead.
func (a *Agent) ForceLeaveOpts(node string, opts ForceLeaveOpts) error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marked

return a.ForceLeaveOptions(node, opts, nil)
}

// ForceLeaveOptions is used to have the agent eject a failed node or remove it
// completely from the list of members. Allows usage of QueryOptions on-top of ForceLeaveOpts
func (a *Agent) ForceLeaveOptions(node string, opts ForceLeaveOpts, q *QueryOptions) error {
r := a.c.newRequest("PUT", "/v1/agent/force-leave/"+node)
r.setQueryOptions(q)
if opts.Prune {
r.params.Set("prune", "1")
}
Expand Down
14 changes: 14 additions & 0 deletions api/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,20 @@ func TestAPI_AgentForceLeavePrune(t *testing.T) {
}
}

func TestAPI_AgentForceLeaveOptions(t *testing.T) {
t.Parallel()
c, s := makeClient(t)
defer s.Stop()

agent := c.Agent()

// Eject somebody with token
err := agent.ForceLeaveOptions(s.Config.NodeName, ForceLeaveOpts{Prune: true}, &QueryOptions{Token: "testToken"})
if err != nil {
t.Fatalf("err: %v", err)
}
}

func TestAPI_AgentMonitor(t *testing.T) {
t.Parallel()
c, s := makeClient(t)
Expand Down