Skip to content

Add the ingester. prefix back to some flags #1413

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 3 commits into from
May 25, 2019

Conversation

csmarchbanks
Copy link
Contributor

@csmarchbanks csmarchbanks commented May 23, 2019

#1258 got rid of the hardcoded ingester. which broke an ingester upgrade for me today. Note that new flags like ruler.consul.hostname will not work after this.

There is only one binary now, so use that in all the k8s/ manifests components.

Changes:

  • Lifecycle flags for the ingester will have the ingester. prefix again
  • ruler.consul.* flags will become consul.*
  • ruler.ring.heartbeat-timeout and ruler.distributor.replication-factor lose the ruler prefix (according to Jacob these are not needed for the ruler anyway)

@csmarchbanks csmarchbanks force-pushed the lifecycle-ingester-prefix branch from 693e231 to 4a6c277 Compare May 23, 2019 23:46
@csmarchbanks csmarchbanks changed the title Keep the ingester. prefix for lifecycle flags Update the demo k8s manifests to reflect some changed images and flags May 23, 2019
@csmarchbanks csmarchbanks force-pushed the lifecycle-ingester-prefix branch from 4a6c277 to 755954b Compare May 24, 2019 01:02
@csmarchbanks csmarchbanks changed the title Update the demo k8s manifests to reflect some changed images and flags Add the ingester. prefix back to some flags; update k8s/ manifests May 24, 2019
@csmarchbanks csmarchbanks force-pushed the lifecycle-ingester-prefix branch from 755954b to fde1f25 Compare May 24, 2019 04:39
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

LGTM.
It's unfortunate to make a breaking change 7 days later, but sounds like few people took this update anyway.
Could you clarify in the PR description the 'from' and 'to' changes as a result of this PR, at least for the bits that did work.

Many ingester flags were recently broken. This fixes the flags that were
broken, but if anyone is running the new HA Ruler they will need to
update their consul flags.

Signed-off-by: Chris Marchbanks <[email protected]>
@csmarchbanks csmarchbanks force-pushed the lifecycle-ingester-prefix branch from 3bb9e66 to b614a3f Compare May 24, 2019 18:06
@csmarchbanks csmarchbanks force-pushed the lifecycle-ingester-prefix branch from b614a3f to dad15af Compare May 24, 2019 18:07
@csmarchbanks csmarchbanks changed the title Add the ingester. prefix back to some flags; update k8s/ manifests Add the ingester. prefix back to some flags May 24, 2019
@tomwilkie
Copy link
Contributor

Sorry about that!

@tomwilkie tomwilkie merged commit 2adbf79 into master May 25, 2019
@tomwilkie tomwilkie deleted the lifecycle-ingester-prefix branch May 25, 2019 17:59
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.

4 participants