Skip to content

add configs api docs #1215

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

Merged
merged 7 commits into from
May 5, 2019
Merged

Conversation

agrahamlincoln
Copy link
Contributor

Signed-off-by: Graham Rounds [email protected]


Wrote this when exploring ruler/configs functionality and figured it would be valuable the official repo. Please correct anything incorrect and/or misleading I may have overlooked.

@tomwilkie
Copy link
Contributor

Thanks @agrahamlincoln, this LGTM! We need a second +1 to merge though. @gouthamve @bboreham @csmarchbanks ?

Copy link
Contributor

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

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

Thanks for the writeup!

I left a couple of comments, in addition, I think both /api/prom/configs/rules and /api/prom/config/alertmanager take/return all the config (all rules files, and alertmanager config).

It's been awhile since I used this API, so sorry if I am not remembering correctly!

@jtlisi
Copy link
Contributor

jtlisi commented Feb 13, 2019

@csmarchbanks It does take a different input. Here is an example:

{
  "new_config": {
    "format_version": "2",
    "files": {
      "example_rules.yaml": "groups:\n  - name: test\n    rules:\n    - record: test_ruler_sum_up\n      expr: sum(up)"
    }
  },
  "old_config": {
    "format_version": "2",
    "files": {
      "example_rules.yaml": ""
    }
  }
}```

@agrahamlincoln agrahamlincoln force-pushed the configs-api-docs branch 2 times, most recently from 44c259a to 071d57e Compare April 23, 2019 03:21
@agrahamlincoln
Copy link
Contributor Author

Thanks for all the feedback! I've added the templates API endpoints, and updated the examples to be accurate. Let me know if there is anything else incorrect or that could use more elaboration.

@rsteneteg
Copy link
Contributor

@agrahamlincoln, I read the updated document, and it sounds like you can send a partial update with only templates or only rules etc.. but that is not correct, each post to any of the endpoints will replace the config with whatever was in the request. So you will need to post everything AM config, rules, templates in each request for all endpoints.

@rsteneteg
Copy link
Contributor

It would however be nice if you could post a partial update with only templates or rules etc, we should create a task to fix that

@bboreham
Copy link
Contributor

bboreham commented May 3, 2019

I think partial updates are listed as part of #619

@bboreham bboreham force-pushed the configs-api-docs branch from 0a235b5 to df8c5b6 Compare May 3, 2019 10:06
@bboreham
Copy link
Contributor

bboreham commented May 3, 2019

I took the liberty of pushing to your branch some updates which I think bring it more into line with what the code implements.

Also I don't think we should include internal APIs in the same doc.

@bboreham
Copy link
Contributor

bboreham commented May 3, 2019

This can probably close #322

@agrahamlincoln
Copy link
Contributor Author

I took the liberty of pushing to your branch some updates which I think bring it more into line with what the code implements.

Thanks! I appreciate you bringing together links to paint some color around why the behavior is what it is today.

@agrahamlincoln
Copy link
Contributor Author

Given that POST requests use the same payload, I consolidated them all into a section and used the extra real-estate to document expected return codes.

I'm pretty comfortable with this doc at this point - but definitely open to any more feedback

@bboreham
Copy link
Contributor

bboreham commented May 3, 2019

Let's merge this now, then any further updates can be a new PR.


`id` - should be incremented every time data is updated; Cortex will use the config with the highest number.

`rule_format_version` - allows compatability for tenants with config in Prometheus V1 format. Pass "1" or "2" according to which Prometheus version you want to match.
Copy link
Contributor

Choose a reason for hiding this comment

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

"compatability" is triggering a CI failure - better fix that or CI will be broken on master

Graham Rounds and others added 7 commits May 3, 2019 16:42
Signed-off-by: Graham Rounds <[email protected]>
Signed-off-by: Graham Rounds <[email protected]>
All APIs take all data types; updated the examples and removed the
implication that this might not be the case.

Added `id` and `config` to examples, and link to Prometheus docs.

Signed-off-by: Bryan Boreham <[email protected]>
Signed-off-by: Bryan Boreham <[email protected]>
Signed-off-by: Bryan Boreham <[email protected]>
Signed-off-by: Graham Rounds <[email protected]>
Copy link
Contributor

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

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

This LGTM now.

@bboreham bboreham merged commit 69269a2 into cortexproject:master May 5, 2019
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

Successfully merging this pull request may close these issues.

7 participants