Skip to content

Conversation

@benjaminhuo
Copy link
Contributor

Signed-off-by: Benjamin [email protected]

Copy link
Member

@tombrk tombrk left a comment

Choose a reason for hiding this comment

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

According to https://kubernetes.io/blog/2019/07/18/api-deprecations-in-1-16/, the apps/v1 API is available since Kubernetes v1.9.

I think it is fair to assume that every user nowadays uses at least this version (otherwise they have probably bigger problems that these jsonnet-libs).

LGTM 🚀

@malcolmholmes malcolmholmes merged commit c567bcf into grafana:master Jan 16, 2020
@andrew-waters
Copy link

@benjaminhuo I'm pretty sure this has broken the memcache lib.

v1 statefulset requires a selector: if you don't add it (and you don't have it in the memcache code here), you'll get a validation error:

kubectl failed exit status 1: error validating data: ValidationError(StatefulSet.spec): missing required field "selector" in io.k8s.api.apps.v1.StatefulSetSpec

I've had a world of pain with Loki today that seems to root from this - and rolling back to $.apps.v1beta1.statefulSet fixes my deployment issues. I think the kube API is automatically adding it in v1beta1 but not in v1.

I may have missed something more obvious though

@tombrk
Copy link
Member

tombrk commented Jan 17, 2020

Very good catch, apologies for having you to figure this out.

I reviewed based on the kubernetes migration doc and it didn't mention this, so I didn't think of this.

You are right, the v1 API requires an explicit selector. Opened an issue: #205

@andrew-waters
Copy link

No worries, thanks for the quick fix. I was wondering how to persist my vendor change 😂

@benjaminhuo
Copy link
Contributor Author

@andrew-waters Sorry for overlooking this and thanks @sh0rez 's quick fix

jeschkies pushed a commit that referenced this pull request Dec 9, 2020
This is useful if your database uses true/false for state and want to make prometheus alerts based on that.
Before, booleans were not able to be parsed.  See issue #201
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