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

apiextensions: fix data race in storage #50098

Merged
merged 1 commit into from
Aug 4, 2017
Merged

apiextensions: fix data race in storage #50098

merged 1 commit into from
Aug 4, 2017

Conversation

nikhita
Copy link
Member

@nikhita nikhita commented Aug 3, 2017 •
edited
Loading

Fixes data race in CRD storage.

Copy to a new map because we cannot write to storageMap without a race as it is used without locking elsewhere.

Release note:

Fix data race during addition of new CRD

/cc @sttts

@k8s-ci-robot k8s-ci-robot requested a review from sttts August 3, 2017 16:14
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 3, 2017
@k8s-ci-robot
Copy link
Contributor

Hi @nikhita. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 3, 2017
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Aug 3, 2017
@nikhita nikhita mentioned this pull request Aug 3, 2017
Merged
13 tasks
@sttts
Copy link
Contributor

sttts commented Aug 3, 2017

👍

@sttts
Copy link
Contributor

sttts commented Aug 3, 2017

For the background: since Go 1.6 concurrent write+read is disallowed and might panic. We do not hit that I guess because our CRD tests never create multiple CRDs.

@sttts
Copy link
Contributor

sttts commented Aug 3, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 3, 2017
@sttts
Copy link
Contributor

sttts commented Aug 3, 2017

/cc @deads2k

@sttts
Copy link
Contributor

sttts commented Aug 3, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 3, 2017
@nikhita
Copy link
Member Author

nikhita commented Aug 3, 2017 •
edited
Loading

Btw this is for addition. For deletion, please see here: 1614a11.

@sttts
Copy link
Contributor

sttts commented Aug 3, 2017

/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhita, sttts

Associated issue requirement bypassed by: sttts

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2017
@nikhita
Copy link
Member Author

nikhita commented Aug 3, 2017

/retest

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 46685, 49863, 50098, 50070, 50096)

@k8s-github-robot k8s-github-robot merged commit c66d249 into kubernetes:master Aug 4, 2017
@wojtek-t wojtek-t added this to the v1.7 milestone Aug 7, 2017
@wojtek-t wojtek-t added cherrypick-candidate release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 7, 2017
@nikhita nikhita mentioned this pull request Aug 7, 2017
Merged
@wojtek-t wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 7, 2017
k8s-github-robot pushed a commit that referenced this pull request Aug 7, 2017
Merge pull request #50250 from nikhita/automated-cherry-pick-of-#5009…
4d5c823 
…8-upstream-release-1.7

Automatic merge from submit-queue

Automated cherry pick of #50098

Cherry pick of #50098 on release-1.7.

#50098: fix data race in storage (during addition)
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

@nikhita nikhita deleted the crd-data-race branch August 9, 2017 12:37
k8s-github-robot pushed a commit that referenced this pull request Aug 30, 2017
Merge pull request #47263 from nikhita/crd-01-validation-types
4457e43 
Automatic merge from submit-queue

apiextensions: validation for customresources

- [x] Add types for validation of CustomResources
- [x] Fix conversion-gen: #49747
- [x] Fix defaulter-gen: kubernetes/gengo#61
- [x] Convert to OpenAPI types
- [x] Validate CR using go-openapi
- [x] Validate CRD Schema
- [x] Add integration tests
- [x] Fix round trip tests: #51204 
- [x] Add custom fuzzer functions
- [x] Add custom conversion functions
- [x] Fix data race while updating CRD: #50098 
- [x] Add feature gate for CustomResourceValidation
- [x] Fix protobuf generation

Proposal: kubernetes/community#708
Additional discussion: #49879, #50625

**Release note**:

```release-note
Add validation for CustomResources via JSON Schema.
```

/cc @sttts @deads2k
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts Awaiting requested review from sttts

@deads2k deads2k Awaiting requested review from deads2k

Assignees

@sttts sttts

@deads2k deads2k

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
7 participants
@nikhita @k8s-ci-robot @sttts @k8s-github-robot @k8s-cherrypick-bot @deads2k @wojtek-t