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.

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

Implement an exhaustive API for Reaper #4

Open
adutra opened this issue May 30, 2021 · 6 comments
Open

Implement an exhaustive API for Reaper #4

adutra opened this issue May 30, 2021 · 6 comments

Comments

@adutra
Copy link
Contributor

adutra commented May 30, 2021

I would like to implement an exhaustive API for Reaper.

The current state of the art is:

  • Some of the exposed REST methods are implemented, mostly in the Cluster and RepairSchedule resources, but not all of the optional parameters are available.
  • A PR Add methods for /repair_run endpoint #3 is open to add an initial support for the RepairRun resource, again not all of the methods and optional parameters are implemented.

Here is my proposal:

  • Complete the existing resources by adding the missing methods and/or parameters, re-using the material in Add methods for /repair_run endpoint #3 for RepairRun methods.
  • Add support for missing resources, and mostly: Snapshot and Node.
  • Add support for authentication.
  • Add tests for new methods reusing the current (and excellent) test environment using Docker.

These things can of course be done little by little, and by different developers.

But I think it would be important to agree on the desired API first.

Indeed, some of the existing API methods and types might need to be renamed or re-engineered to comply with things like:

  • consistent naming of types, and methods and parameters
  • consistent mapping of REST endpoints to Go methods
  • how to expose optional parameters
  • consistent behavior (consistent error messages, etc.)

Also, I think it is important to settle on a common code related to HTTP plumbery, so that new methods can be implemented faster and more reliably. This will likely require some refactoring of the existing HTTP-related functions.

I took the initiative to bootstrap a design document with a proposal for a revised API:

https://docs.google.com/document/d/1xau9Lr9BsctZBgpoFnWJmmkFz5c885O8MhqkGWMca1I/edit?usp=sharing

Feel free to add comments there or here.

If/when we agree on the proposed changes, I can start working on this ASAP and provide implementations for the most important resources as a first step: Login, Ping, Cluster, RepairRun and RepairSchedule.

┆Issue is synchronized with this Jiraserver Task by Unito
┆Issue Number: K8SSAND-496
┆Priority: Medium

@burmanm
Copy link
Contributor

burmanm commented May 31, 2021

I'm struggling with one major thing: Why? Is there even a ticket which explains why these are needed?

The reaper-client-go was only ever intended to be used for testing of reaper-operator features. None of these enhancements make a difference in that sense.

@adutra
Copy link
Contributor Author

adutra commented May 31, 2021

Why?

Maybe for nothing :-) I just thought it would be cool for Reaper to have an official Go client, but maybe I'm overthinking all of this.

None of these enhancements make a difference in that sense.

We would need at least a minimal RepairRun and RepairSchedule support in order to implement k8ssandra/k8ssandra#580.

@burmanm
Copy link
Contributor

burmanm commented May 31, 2021

Yes, RepairRun / RepairSchedule (like #3 started) would be necessary to test that scheduling was done properly. But otherwise, I personally think Reaper is something we wish the user would never have to worry about or even think about.

@jdonenine
Copy link
Collaborator

From the K8ssandra perspective I do think we want Reaper to be something that users don't have to think too much about, so I agree with you there @burmanm

Looking at it a bit more broadly though it does seem like a useful piece for the broader Reaper community, which has plenty of bulk outside of K8ssandra. I'm also thinking that this could prove fairly useful if there were to be a K8ssandra UI/API/client that could leverage this as a good clean API integration point.

There is however a lot here, we'd need to think about how to prioritize things and see what might have the most value in the K8ssandra context.

Super nice doc @adutra thanks for putting the thought and time into this idea.

@adutra
Copy link
Contributor Author

adutra commented Jun 1, 2021

We would need at least a minimal RepairRun and RepairSchedule support in order to implement k8ssandra/k8ssandra#580.

Correcting myself: we don't need repair schedules currently, only repair runs. I listed the 3 calls that need replacement here: k8ssandra/k8ssandra#580 (comment).

With that in mind, I think we could start by providing just the RepairRun resource implementation. Others can definitely wait.

@adutra
Copy link
Contributor Author

adutra commented Jun 1, 2021

As discussed today with the team, I'm deprecating the Google doc mentioned above in favor of a PR to discuss the API design: #5 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants