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

Release 1.0.0 #227

Merged
merged 24 commits into from
Nov 25, 2021
Merged

Release 1.0.0 #227

merged 24 commits into from
Nov 25, 2021

Conversation

nschad
Copy link
Collaborator

@nschad nschad commented Sep 22, 2021

Release 1.0.0

  • Allow overriding cortex Version
  • Update Policy API Version
  • Deprecate *.persistence
  • Remove clusterPort for alertmanager
  • Remove legacy alertmanager clustering
  • Removed unused nginx.serviceMonitor section
  • Added IngressClass to ingressResource
  • Update default values for bitnami memcached chart
  • Drop default chunk storage config (the chart still fully supports chunk storage if you so choose too)
  • server.grpc_server_max_recv_msg_size: 104857600 -> 10485760
  • server.grpc_server_max_send_msg_size: 104857600 -> 10485760
  • server.grpc_server_max_concurrent_streams: 1000 -> 10000
  • server.ingester_client.grpc_client_config.max_recv_msg_size: 104857600 -> 10485760
  • server.ingester_client.grpc_client_config.max_send_msg_size: 104857600 -> 10485760
  • config.limits.enforce_metric_name: false -> true
  • config.ruler.enable_api: false -> true
  • configure memberlist automatically
  • config.ingester.lifecycler.final_sleep: 0s -> 30s

Which issue(s) this PR fixes:
Fixes #202, #252

Checklist

  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@nschad nschad marked this pull request as draft September 22, 2021 18:35
@nschad nschad mentioned this pull request Sep 23, 2021
@cabrinha
Copy link
Collaborator

Let's also add the ability to deploy the config as a configMap rather than a secret?

@nschad
Copy link
Collaborator Author

nschad commented Sep 29, 2021

Let's also add the ability to deploy the config as a configMap rather than a secret?

And have the credentials passed as environment variables backed by secrets? That would be quite optimal.

Chart.yaml Outdated Show resolved Hide resolved
values.yaml Show resolved Hide resolved
templates/alertmanager/alertmanager-dep.yaml Outdated Show resolved Hide resolved
Comment on lines +1521 to +1371
- name: MEMCACHED_CACHE_SIZE
value: "1024"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to correspond to the memory requests and limits. It shouldn't just arbitrarily be 1024 MB. It would be better to document how to do this than to hardcode it.

Copy link
Collaborator Author

@nschad nschad Oct 1, 2021

Choose a reason for hiding this comment

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

Yep it should also match with stuff like..

      # CLI flag: -blocks-storage.bucket-store.index-cache.memcached.max-item-size
      [max_item_size: <int> | default = 1048576]

Copy link
Collaborator

@kd7lxl kd7lxl Oct 1, 2021

Choose a reason for hiding this comment

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

The CACHE_SIZE contains all the items, so it does not directly relate to max_item_size, other than having to be bigger than it to fit at least one item.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've actually had a lot of issues figuring out which components of Cortex require what item sizes in the different caches. It'd be great if the Cortex documentation offered more clarity on what the max size an item could be, like an index, some metadata, or a block even.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The CACHE_SIZE contains all the items, so it does not directly relate to max_item_size, other than having to be bigger than it to fit at least one item.

Yep u right. Leaving this here for future reference: https://github.com/bitnami/bitnami-docker-memcached/blob/master/1/debian-10/rootfs/opt/bitnami/scripts/memcached/run.sh

Copy link
Collaborator Author

@nschad nschad Oct 5, 2021

Choose a reason for hiding this comment

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

I documented the helm values but we should have a Best-Practice wiki page on how to configure memcached properly with some (preferably) production experience. Doesn't have to be in this PR though

@lahsivjar
Copy link
Contributor

Any ETA on 1.0.0 release? Is it possible to tag the current master? We have some fixes on the Memcache chart that we want to apply to our workload.

@kd7lxl
Copy link
Collaborator

kd7lxl commented Oct 4, 2021

@ShuzZzle, I'd recommend breaking up this PR so that it only contains the breaking changes. Before it's ready, we can release a minor version with non-breaking updates and new features.

Edit: never mind, I see this PR is already only the breaking changes. @lahsivjar comment threw me off because it's out of context. Sounds like they are requesting a minor version release (nothing to do with this PR).

@nschad
Copy link
Collaborator Author

nschad commented Oct 4, 2021

Any ETA on 1.0.0 release? Is it possible to tag the current master? We have some fixes on the Memcache chart that we want to apply to our workload.

The same memcache fixxes in this PR or something similar?

@lahsivjar
Copy link
Contributor

lahsivjar commented Oct 4, 2021

@kd7lxl @ShuzZzle Sorry for the confusion. I was not referring to this specific PR but releasing the current master with a minor tag. I need the changes done in liveliness probes in Memcached chart version 5.15.1. Should have opened a separate thread 😅

@nschad
Copy link
Collaborator Author

nschad commented Oct 4, 2021

@kd7lxl @ShuzZzle Sorry for the confusion. I was not referring to this specific PR but releasing the current master with a minor tag. I need the changes done in liveliness probes in Memcached chart version 5.15.1. Should have opened a separate thread 😅

Sounds great, please do before we release a "buggy" version

@nschad nschad marked this pull request as ready for review October 5, 2021 09:13
Chart.yaml Show resolved Hide resolved
@nschad nschad mentioned this pull request Oct 19, 2021
1 task
@cabrinha cabrinha requested review from cabrinha and kd7lxl October 26, 2021 17:00
@nschad nschad force-pushed the release-1.0.0 branch 2 times, most recently from ddd930a to 9f84719 Compare November 5, 2021 10:42
@nschad nschad linked an issue Nov 17, 2021 that may be closed by this pull request
@nschad nschad force-pushed the release-1.0.0 branch 2 times, most recently from bafd8a7 to cb48943 Compare November 21, 2021 10:58
Comment on lines +12 to +14
{{- if .Values.ingress.ingressClass.enabled }}
ingressClassName: {{ .Values.ingress.ingressClass.name }}
{{- end }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
…rride

Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
Signed-off-by: ShuzZzle <niclas.schad@gmail.com>
@nschad nschad merged commit ad6ebfd into cortexproject:master Nov 25, 2021
@nschad nschad deleted the release-1.0.0 branch November 25, 2021 11:54
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.

Tag 0.7.0 does not include PR 242 policy/v1beta1
4 participants