Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Do writes in init container before trying reads #73318

Merged
merged 1 commit into from
Feb 5, 2019
Merged

Do writes in init container before trying reads #73318

merged 1 commit into from
Feb 5, 2019

Conversation

wongma7
Copy link
Contributor

@wongma7 wongma7 commented Jan 25, 2019 •
edited

What type of PR is this?
/kind flake

What this PR does / why we need it: In some subpath tests, it is expected that container 1 reads something container 0 writes, but there is no guarantee that container 0 starts before 1 (or vice versa). The read check is 10 seconds so if container 0 starts 10s after container 1, the test will fail. To work around this, make init versions of container 0 and 1 and always use the init version when writing.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

/sig storage

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/flake Categorizes issue or PR as related to a flaky test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/storage Categorizes an issue or PR as relevant to SIG Storage. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jan 25, 2019
@wongma7 wongma7 force-pushed the subpath-init-write branch 3 times, most recently from eb61f6e to 93d4d65 Compare January 25, 2019 21:21
@wongma7
Copy link
Contributor Author

wongma7 commented Jan 28, 2019

/retest

1 similar comment
@wongma7
Copy link
Contributor Author

wongma7 commented Jan 28, 2019

/retest

@wongma7
Copy link
Contributor Author

wongma7 commented Jan 28, 2019

/retest
gce:
W0128 17:16:53.436] - The zone 'projects/k8s-gce-ingress1-5/zones/us-central1-f' does not have enough resources available to fulfill the request. '(resource type:pd-standard)'.
aws:
W0128 16:13:03.119] 2019/01/28 16:13:03 main.go:315: Something went wrong: starting e2e cluster: waiting for ready nodes timed out

@wongma7
Copy link
Contributor Author

wongma7 commented Jan 30, 2019

/test pull-kubernetes-e2e-kops-aws

@wongma7
Copy link
Contributor Author

wongma7 commented Jan 30, 2019

/retest

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jan 30, 2019 •
edited

@wongma7: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 9f5a799 link /test pull-kubernetes-e2e-kops-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@wongma7 wongma7 changed the title WIP: Do writes in init container before trying reads Do writes in init container before trying reads Jan 30, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2019
@wongma7
Copy link
Contributor Author

wongma7 commented Jan 30, 2019

@kubernetes/sig-storage-pr-reviews

@wongma7 wongma7 mentioned this pull request Jan 30, 2019
Closed
gnufied
gnufied reviewed Jan 30, 2019
View reviewed changes
test/e2e/storage/testsuites/subpath.go
SecurityContext: &v1.SecurityContext{
Privileged: &privilegedSecurityContext,
},
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need 2 regular containers in pod?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, looks like other test depend on them.

@gnufied
Copy link
Member

gnufied commented Jan 30, 2019

/lgtm

cc @msau42 for approval.

@gnufied
Copy link
Member

gnufied commented Jan 30, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2019
@gnufied
Copy link
Member

gnufied commented Jan 30, 2019

/assign @msau42

@msau42
Copy link
Member

msau42 commented Feb 4, 2019

/approve
/retest

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, wongma7

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 4, 2019
@k8s-ci-robot k8s-ci-robot merged commit 641b2ad into kubernetes:master Feb 5, 2019
@wongma7 wongma7 mentioned this pull request Feb 5, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnufied gnufied gnufied left review comments

@msau42 msau42 Awaiting requested review from msau42

@saad-ali saad-ali Awaiting requested review from saad-ali

Assignees

@gnufied gnufied

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/flake Categorizes issue or PR as related to a flaky test. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wongma7 @k8s-ci-robot @gnufied @msau42