-
Notifications
You must be signed in to change notification settings - Fork 21
bump/add new revisions for opsportal/kommander charts #100
Conversation
1f6259f
to
dd5e4e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
checked against mesosphere/charts
https://github.com/mesosphere/charts/blob/master/stable/opsportal/Chart.yaml#L6
and
https://github.com/mesosphere/charts/blob/master/stable/kommander/Chart.yaml#L10
hm, not sure what the CI issue is about:
Does this mean the test "configmap/local-path-config created" has timed out? 🤔 |
The error output in this (very young) CI code is very lacking right now. This error means that the test failed because the addons could not fully provision. We intend to follow up on this and improve the experience as per DCOS-61626, but until then its customary to run the tests locally and monitor the addons to determine the source of the issue. |
running tests locally
so correct me if I am wrong, this is not related to kommander itself at all, but part of the testing process. right? |
Looks like a problem in the underlying chart which we've just now caught. Since this didn't occur in previous CI runs it would seem that a recent chart update introduced this problem? That being the case then CI would appear to have done it's job and caught a problem and we should fix the chart and update this PR with a chart version that deploys and tears down fully. Let me know if you feel there's any context I've missed here? |
didn't checked this karma thing ... will do now |
04dcc6d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When CI is happy, I'm happy 👍
f1ac89d
to
fb7d991
Compare
rebased, seems like opsportal got merged in a separate PR, CI for kommander still being red. I cant make a lot of sense of that CI issue :( |
e73da87
to
22b8e45
Compare
Ok, np I'll take a look at the CI issue today @juliangieseke and get it sorted out 👍 |
Thanks for the heads up, I'm going to put this back in |
@shaneutt @juliangieseke Let's see if getting mesosphere/charts#386 merged and bumping to that chart version here fixes it? I'm not sure why the cleanup job is failing now and not before, but there are some rbac improvements in that PR that could fix this. |
it was the same error for me #100 (comment) |
@sebbrandt87 ah ok, I've been under the impression that mesosphere/charts#384 should have fixed the issue… |
mesosphere/charts#384 are changes to kommander-karma and kommander-thanos charts, which are dependencies in the kommander chart - those deps are actually bumped in mesosphere/charts#386 so the changes wouldn't be in until that PR is merged and we bump the version of kommander here |
@juliangieseke just merged mesosphere/charts#386 - let's update the kommander chart version to 0.3.21 and see if it fixes things |
thanks for heads up @gracedo lets wait for CI 🤞 |
91d69a1
to
71d178e
Compare
CI run to have a look at: https://teamcity.mesosphere.io/viewLog.html?buildId=2566529&buildTypeId=kubeaddons_KonvoyAddons (Ive bumped ui again to include a fix) |
bumping kommander & ops portal charts by adding new revisions.
I made two commits to keep changes visible, first commit is duplicating files, second commit does the change: dd5e4e7
@shaneutt last double checking that this is correct, mind having a look? 🙇