-
Notifications
You must be signed in to change notification settings - Fork 38.8k
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
Test: Create multivolume StatefulSet test #41180
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED The following people have approved this PR: justinsb Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
Oh, in case anyone was feeling really keen... let's not merge this until we've got a fix for #35695, as I think we might have multizone tests on GKE! |
3c657ba
to
4f10b01
Compare
And I manually tested this and it failed with the expected error (although it looks like the master branch fails against k8s 1.5). The test failures above are because of the gcloud problem though. So once #40910 has merged, we can consider this one! |
We create a StatefulSet with two volumes per pod. We reuse the zookeeper StatefulSet, but we don't currently use the second volume. That will need to be fixed in the zookeeper image in the contrib repo. However, this should be enough to detect issue kubernetes#35695.
4f10b01
to
2c07a1d
Compare
Removing issue from 1.6 milestone since code freeze has begun:
/cc @kubernetes/release-team @kubernetes/kubernetes-release-managers |
It's a test for a bugfix that merged in 1.6. Marking as |
I'm cleaning out the milestone since we're coming up on the cut. This doesn't seem to be a release-blocker. |
ping @smarterclayton |
@kow3ns as well |
ping @smarterclayton |
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.
If the point is to verify that you can use multiple Volumes in a StatefulSet why not do a simple application that actually reads are write to multiple volumes instead of using a ZooKeeper that writes its snapshots and WAL to one Volume an nothing to the other.
labels: | ||
app: multivolumezk | ||
annotations: | ||
pod.alpha.kubernetes.io/init-containers: '[ |
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.
If you are targeting >= 1.6 you should be using the fields instead of the annotations.
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 like they all need to be updated, which is a bigger PR. Opened #45202
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.
Switch this one here, the rest of the examples can be handled as part of that issue (I have already seen PRs switching to fields).
This is merely a test for the bug in #35695. I agree we should continue to add tests that test more functionality, but one PR at a time. |
ping @kow3ns @smarterclayton |
ping @kow3ns @smarterclayton Can you please progress or reassign? |
@k8s-bot pull-kubernetes-federation-e2e-gce test this |
ping @kow3ns @smarterclayton Can you please progress or reassign? |
relabeling sig per https://github.com/kubernetes/kubernetes/blob/master/test/test_owners.json#L30 |
/retest |
/remove-priority P1 |
@justinsb PR needs rebase |
This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review. You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days |
We create a StatefulSet with two volumes per pod. We reuse the
zookeeper StatefulSet, but we don't currently use the second volume.
That will need to be fixed in the zookeeper image in the contrib repo.
However, this should be enough to detect issue #35695.