Add DataSource and TypedLocalObjectReference #67087

xing-yang · 2018-08-07T16:54:28Z

What this PR does / why we need it:
This PR adds TypedLocalObjectReference in the core API and adds DataSource in PersistentVolumeClaimSpec.

It also enables feature gate for VolumeSnapshotDataSource.

This is part of the CSI snapshot design proposal to support restoring a volume from a snapshot:
kubernetes/community#2495

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
kubernetes/enhancements#177

Special notes for your reviewer:

Release note:

Added support to restore a volume from a volume snapshot data source.

xing-yang · 2018-08-07T16:55:50Z

@jingxu97

xing-yang · 2018-08-07T16:56:19Z

@saad-ali

neolit123 · 2018-08-07T23:16:19Z

/ok-to-test
/sig api-machinery
/sig storage

fedebongio · 2018-08-09T20:13:13Z

/assign @lavalamp

This PR adds changes to support restoring a volume from a snapshot. Note: Mark it WIP until the DataSource addition in core API (kubernetes/kubernetes#67087) and snapshot API/controller changes (kubernetes-csi/external-snapshotter#7) are merged.

saad-ali · 2018-08-20T17:26:20Z

/lgtm
Looks good from sig-storage side.
Needs API Approval. CC @kubernetes/api-approvers
/assign @thockin

thockin

This PR is missing the readiness gate portion. Do you really want to merge this without that?

The risk is that you don't get the readiness done in time, and the release cuts with a half-cooked feature. We're trying to do less in-progress development in master to avoid this problem.

Should this wait for the readiness feature to be mergeable? Either as part of this PR or as a pre-req for this.

thockin · 2018-08-21T17:25:43Z

pkg/apis/core/types.go

+	Name string
+	// Kind of the referent.
+	// +optional
+	Kind string


@lavalamp @kubernetes/sig-api-machinery-api-reviews I need a consult.

Is "kind" sufficient here? In general a kind holds something like "Endpoints" while apiVersion holds "extensions/v1beta1". In a context like this, I don't care which API version, but I do care which group.

kind, API group, and name are the three bits of info needed to unambiguously reference a particular object (assuming you don't care what API version the thing chasing the reference chooses to use).

for example:

kubernetes/staging/src/k8s.io/api/rbac/v1/types.go

Lines 90 to 98 in 7749eea

// RoleRef contains information that points to the role being used

type RoleRef struct {

// APIGroup is the group for the resource being referenced

APIGroup string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`

// Kind is the type of resource being referenced

Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`

// Name is the name of resource being referenced

Name string `json:"name" protobuf:"bytes,3,opt,name=name"`

}

Thanks, Jordan, that's what I assumed. I saw that ObjectReference has a field "APIVersion" which seemed wrong... I didn't know this struct, but it makes more sense.

Added APIGroup.

Then we will add the APIGroup in the struct. I assumed it is required and user has to specify the APIGroup here for the data source.

xing-yang · 2018-08-21T17:36:14Z

@thockin We have another PR on the readiness gate depending on this PR: #67625. Do you want us to combine them into one PR? Thanks.

xing-yang · 2018-08-28T15:51:38Z

The pull-kubernetes-kubemark-e2e-gce-big test failure was not related to this PR. I'll re-run it.
Looks like another feature gate change just got merged, so I'll need to resolve the conflict and re-submit.

xing-yang · 2018-08-28T18:43:37Z

@thockin Jing and I discussed about this and want to make the following changes to the DataSource comments. Let us know what you think. Thanks!

// This field requires the VolumeSnapshotDataSource alpha feature gate to be enabled and
// currently VolumeSnapshot is the only supported data source. If the provisioner can support
// VolumeSnapshot data source, it will create a new volume and data will be restored to the volume
// at the same time. If the provisioner does not support VolumeSnapshot data source, the volume
// will not be created. In the future, we plan to support more data source types and the behavior of
// the provisioner may change.

jingxu97 · 2018-08-28T18:57:20Z

@thockin The comment we have now only describes the provisioner behavior for the only supported data source type, VolumeSnapshot. For this case, the provisioner will create volume and put data in volume at the same time. For the data sources that will be added in the future, it might require an external data populator to handle the data copy. In this case, things will get more complicated. For example, although provisioner does not fully support the data source (it cannot copy data directly), it still creates the volume and let the data populator to handle the data copying. In case of data copy failure, an empty volume might be left there but PVC should be marked as not ready. We need more discussion to finalize the design and cover all the cases. When that feature is ready, we will come back and change the comments here. Please let us know if there is any problem with it. Thank you!

This PR adds TypedLocalObjectReference in the core API and adds DataSource in PersistentVolumeClaimSpec.

xing-yang · 2018-08-28T22:08:48Z

/test pull-kubernetes-e2e-gce

xing-yang · 2018-08-28T22:09:00Z

/test pull-kubernetes-kubemark-e2e-gce-big

thockin · 2018-08-29T05:39:32Z

/lgtm
/approve

saad-ali

/lgtm
/approve

k8s-ci-robot · 2018-08-29T17:22:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saad-ali, thockin, xing-yang

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:

~~api/OWNERS~~ [thockin]
~~docs/api-reference/OWNERS~~ [thockin]
~~pkg/apis/core/OWNERS~~ [thockin]
~~pkg/features/OWNERS~~ [saad-ali,thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

saad-ali · 2018-08-29T17:22:33Z

/milestone v1.12

k8s-github-robot · 2018-08-29T17:59:04Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-08-29T19:08:28Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

This PR adds changes to support restoring a volume from a snapshot. Note: The DataSource addition in core API (kubernetes/kubernetes#67087) and snapshot API/controller changes (kubernetes-csi/external-snapshotter#7) are merged.

k8s-ci-robot requested review from davidopp and krousey August 7, 2018 16:54

k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Aug 7, 2018

xing-yang mentioned this pull request Aug 7, 2018

WIP: Restore Volume from Snapshot kubernetes-csi/external-provisioner#119

Closed

k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 7, 2018

xing-yang force-pushed the datasource branch from eb16586 to adce0a8 Compare August 9, 2018 15:03

k8s-ci-robot assigned lavalamp Aug 9, 2018

xing-yang mentioned this pull request Aug 10, 2018

Restore Volume from Snapshot kubernetes-csi/external-provisioner#123

Merged

xing-yang force-pushed the datasource branch 2 times, most recently from 61fc09a to 0a59a42 Compare August 16, 2018 20:16

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2018

xing-yang force-pushed the datasource branch from 0a59a42 to 6e41803 Compare August 19, 2018 13:47

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2018

k8s-ci-robot assigned saad-ali and thockin Aug 20, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2018

thockin reviewed Aug 21, 2018

View reviewed changes

xing-yang force-pushed the datasource branch from 31cf382 to 289e1e2 Compare August 28, 2018 17:20

xing-yang added 11 commits August 28, 2018 13:23

Add TypedLocalObjectReference and DataSource

e4c82b3

This PR adds TypedLocalObjectReference in the core API and adds DataSource in PersistentVolumeClaimSpec.

Add feature gate for VolumeSnapshotDataSource

2a442db

Add APIGroup to TypedLocalObjectReference

34f62ae

Add validation for feature gate

e4034e2

Add generated files

2a9cff2

Address Tim's comments

5a6c5b0

Modify comments for DataSource

46fdfeb

Clarify comments for DataSource

2e4f6ec

Modify comments for DataSource

9c1b29e

Modify DataSource comments

de00a66

Re-generate files

88d3d96

xing-yang force-pushed the datasource branch from 289e1e2 to 88d3d96 Compare August 28, 2018 20:57

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 29, 2018

saad-ali approved these changes Aug 29, 2018

View reviewed changes

k8s-ci-robot added this to the v1.12 milestone Aug 29, 2018

k8s-github-robot merged commit d97ece0 into kubernetes:master Aug 29, 2018

xing-yang mentioned this pull request Jan 21, 2019

REQUEST: New membership for xing-yang kubernetes/org#393

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DataSource and TypedLocalObjectReference #67087

Add DataSource and TypedLocalObjectReference #67087

xing-yang commented Aug 7, 2018 •

edited

Loading

xing-yang commented Aug 7, 2018

xing-yang commented Aug 7, 2018

neolit123 commented Aug 7, 2018

fedebongio commented Aug 9, 2018

saad-ali commented Aug 20, 2018

thockin left a comment

thockin Aug 21, 2018

liggitt Aug 21, 2018 •

edited

Loading

thockin Aug 21, 2018

xing-yang Aug 21, 2018

jingxu97 Aug 21, 2018

xing-yang commented Aug 21, 2018

xing-yang commented Aug 28, 2018

xing-yang commented Aug 28, 2018 •

edited by jingxu97

Loading

jingxu97 commented Aug 28, 2018

xing-yang commented Aug 28, 2018

xing-yang commented Aug 28, 2018

thockin commented Aug 29, 2018

saad-ali left a comment

k8s-ci-robot commented Aug 29, 2018

saad-ali commented Aug 29, 2018

k8s-github-robot commented Aug 29, 2018

k8s-github-robot commented Aug 29, 2018

	// RoleRef contains information that points to the role being used
	type RoleRef struct {
	// APIGroup is the group for the resource being referenced
	APIGroup string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`
	// Kind is the type of resource being referenced
	Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
	// Name is the name of resource being referenced
	Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
	}

Add DataSource and TypedLocalObjectReference #67087

Add DataSource and TypedLocalObjectReference #67087

Conversation

xing-yang commented Aug 7, 2018 • edited Loading

xing-yang commented Aug 7, 2018

xing-yang commented Aug 7, 2018

neolit123 commented Aug 7, 2018

fedebongio commented Aug 9, 2018

saad-ali commented Aug 20, 2018

thockin left a comment

Choose a reason for hiding this comment

thockin Aug 21, 2018

Choose a reason for hiding this comment

liggitt Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

thockin Aug 21, 2018

Choose a reason for hiding this comment

xing-yang Aug 21, 2018

Choose a reason for hiding this comment

jingxu97 Aug 21, 2018

Choose a reason for hiding this comment

xing-yang commented Aug 21, 2018

xing-yang commented Aug 28, 2018

xing-yang commented Aug 28, 2018 • edited by jingxu97 Loading

jingxu97 commented Aug 28, 2018

xing-yang commented Aug 28, 2018

xing-yang commented Aug 28, 2018

thockin commented Aug 29, 2018

saad-ali left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 29, 2018

saad-ali commented Aug 29, 2018

k8s-github-robot commented Aug 29, 2018

k8s-github-robot commented Aug 29, 2018

xing-yang commented Aug 7, 2018 •

edited

Loading

liggitt Aug 21, 2018 •

edited

Loading

xing-yang commented Aug 28, 2018 •

edited by jingxu97

Loading