-
Notifications
You must be signed in to change notification settings - Fork 316
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
Truncate the statefulset pvc name to be max 63 chars #799
Conversation
Hmm not sure why the test-control-plane test failed. I ran |
Hi @tjhiggins I reran the tests from the failure, it looks like it was a unit test that failed that prevented the acceptance tests from running. Thank you for the contribution! |
Which unit test? The only failing test I see now is the dev-upload-docker. Confused if I need to do anything. |
@tjhiggins There was a unit test that failed so I re-ran the tests from CircleCI starting from that failure. The unit tests can be flaky sometimes. I believe our dev uploads are currently failing which is ok. We'd need someone probably to take a look and review this PR formally, we've been a little behind on PR reviews due to our beta release this week. Could you perhaps show us what the output looks like for |
tj:consul (798-truncate-statefulset-pvc)$ kubectl get pvc
NAME STATUS VOLUME CAPACITY ACCESS MODES STORAGECLASS AGE
data-default-consul-consul-server-0 Bound pvc-fd364619-f97d-4cac-8b97-b8a6cc3009fd 10Gi RWO hostpath 53s
data-default-consul-consul-server-1 Bound pvc-7cf89cf1-2f89-4602-9943-78929b2229c4 10Gi RWO hostpath 53s
data-default-consul-consul-server-2 Bound pvc-5405d834-e29b-4516-9026-3907e5b449bd 10Gi RWO hostpath 53s |
* Don't run them in parallel so that we only get one slack notification * Add gotestsum summary
Looks great! Can you write a Helm test for it? |
@lkysow done |
@test "server/StatefulSet: truncation for namespace > 58 chars" { | ||
cd `chart_dir` | ||
local actual=$(helm template \ | ||
-n really-really-really-really-really-really-really-long-namespace \ |
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.
😄
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.
Looks great!! Thanks so much.
Changes proposed in this PR:
How I expect reviewers to test this PR: