Sorted the container arguments inside deployments #337
Conversation
|
This change is |
|
With this PR, the collector deployment looks stable: $ kubectl get pods
NAME READY STATUS RESTARTS AGE
elasticsearch-0 1/1 Running 0 48m
jaegerqe-collector-58bd8db845-94x9b 1/1 Running 0 4m
jaegerqe-collector-58bd8db845-9px2r 1/1 Running 0 4m
jaegerqe-collector-58bd8db845-gl9dl 1/1 Running 0 4m
jaegerqe-collector-58bd8db845-jrn67 1/1 Running 0 4m
jaegerqe-query-7b9d5975bf-v4cr9 2/2 Running 0 4m
$ kubectl get deployments -o yaml jaegerqe-collector | grep deployment.kubernetes.io/revision
deployment.kubernetes.io/revision: "1" |
jpkrohling
changed the title
|
@jpkrohling Looks like it has affected some other tests |
|
@jpkrohling Did you test on openshift aswell? Or was the instability also on k8s? |
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
jpkrohling
force-pushed
the
334-Sort-deployment-arguments
branch
from
30226cb to
16fa0d3
Compare
|
I tested this on OpenShift only, I did not notice any instability in Kubernetes. I just amended the commit, to include the containers in the |
Codecov Report
@@ Coverage Diff @@
## master #337 +/- ##
==========================================
+ Coverage 89.71% 89.73% +0.02%
==========================================
Files 64 64
Lines 2985 2991 +6
==========================================
+ Hits 2678 2684 +6
Misses 207 207
Partials 100 100
Continue to review full report at Codecov.
|
jpkrohling
changed the title
| "--tls-key=/etc/tls/private/tls.key", | ||
| "--upstream=http://localhost:16686", | ||
| } | ||
|
|
There was a problem hiding this comment.
Would it be better to add the sort, so not relying on users to read the comment?
There was a problem hiding this comment.
Although I've just seen the test, so that should pick it up.
There was a problem hiding this comment.
Thought about it, but as this is a static list, it might be better to enforce via test and spare a couple of CPU cycles :-)
Fixes #334
Signed-off-by: Juraci Paixão Kröhling [email protected]