Update kube-thanos #741
Update kube-thanos #741
Conversation
openshift-ci-robot
added
the
approved
|
Your generate test is failing 😝 think you need run the make generate command. |
Also realized this, was already running 🙂 . Done. |
| @@ -37,7 +59,7 @@ spec: | |||
| - if [ -x "$(command -v curl)" ]; then curl http://localhost:9090/-/healthy; | |||
| elif [ -x "$(command -v wget)" ]; then wget --quiet --tries=1 --spider | |||
| http://localhost:9090/-/healthy; else exit 1; fi | |||
| name: thanos-querier | |||
| name: thanos-query | |||
There was a problem hiding this comment.
Note this is used and should be changed here as well https://github.com/openshift/cluster-monitoring-operator/blob/master/pkg/manifests/manifests.go#L2605
seems like tests fail 🤔 |
|
Note there is a flake around our e2e tests, which might be the failure is about. |
|
These failures don't involve port-forwarding. /retest |
| app.kubernetes.io/name: thanos-querier | ||
| app.kubernetes.io/component: query-layer | ||
| app.kubernetes.io/instance: thanos-querier | ||
| app.kubernetes.io/name: thanos-query |
There was a problem hiding this comment.
deployment name is still thanos-querier, I expect the name to reflect that, wdyt?
There was a problem hiding this comment.
name is the kubernetes app name (instance already reflects the name of this particular instance), as kube-thanos is the defining entity here, I don't think we should change this, we should however eventually rename thanos-querier to thanos-query
There was a problem hiding this comment.
we should however eventually rename thanos-querier to thanos-query
Agreed, can you open jira for this? Thanks! :)
| @@ -2602,7 +2602,7 @@ func (f *Factory) ThanosQuerierDeployment(grpcTLS *v1.Secret, enableUserWorkload | |||
|
|
|||
| if f.config.ThanosQuerierConfig.Resources != nil { | |||
| for i, c := range d.Spec.Template.Spec.Containers { | |||
| if c.Name == "thanos-querier" { | |||
| if c.Name == "thanos-query" { | |||
There was a problem hiding this comment.
We have this in the unit tests as well. Can you also add we should error out if that container name is not there, thanks! Or we can do that in follow up as well. Lets just adjust that test, thanks!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brancz, lilic The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR updates the kube-thanos dependency to latest
masterand consistently uses the Kubernetes recommended app labels for the Thanos query component.@openshift/openshift-team-monitoring