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.

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

Populate Staging repo csi-translation-lib #72770

Merged
merged 1 commit into from Jan 15, 2019

Conversation

@ddebroy
Copy link
Member

ddebroy commented Jan 10, 2019

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test

/kind feature

/kind flake

What this PR does / why we need it:
We need the CSI migration library code in a staging repo to avoid cyclical go dependencies between in-tree code and the migration library code while allowing external CSI repos to make use of the common library code. This PR populates the staging repo csi-translation-lib with the contents of kubernetes-csi/kubernetes-csi-migration-library (which will be removed).

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
Related to: kubernetes/org#321

Does this PR introduce a user-facing change?:

NONE

/sig storage
/cc @davidz627 @leakingtapan

@ddebroy

This comment has been minimized.

Copy link
Member Author

ddebroy commented Jan 10, 2019

/assign @smarterclayton

@@ -0,0 +1,96 @@
/*
Copyright 2018 The Kubernetes Authors.

This comment has been minimized.

Copy link
@nikhita

nikhita Jan 10, 2019

Member

2019 :D

@nikhita

This comment has been minimized.

Copy link
Member

nikhita commented Jan 10, 2019

Some staging-related changes:

@ddebroy

This comment has been minimized.

Copy link
Member Author

ddebroy commented Jan 11, 2019

/retest

@nikhita nikhita referenced this pull request Jan 11, 2019

Merged

Add csi-translation-lib repo #145

2 of 2 tasks complete
@ddebroy

This comment has been minimized.

Copy link
Member Author

ddebroy commented Jan 11, 2019

/retest

@sttts

This comment has been minimized.

Copy link
Contributor

sttts commented Jan 11, 2019

This needs an entry in hack/import-restrictions.yaml.

@leakingtapan

This comment has been minimized.

Copy link
Contributor

leakingtapan commented Jan 11, 2019

/lgtm

Populate CSI translation library staging repo
Signed-off-by: Deep Debroy <ddebroy@docker.com>

@ddebroy ddebroy force-pushed the ddebroy:ddebroy-csi-translation-1 branch from d3d76f9 to 913bd97 Jan 11, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jan 11, 2019

@ddebroy

This comment has been minimized.

Copy link
Member Author

ddebroy commented Jan 11, 2019

Squashed all commits for final review/approval.

@davidz627

This comment has been minimized.

Copy link
Contributor

davidz627 commented Jan 12, 2019

/lgtm
Thanks deep!

@ddebroy

This comment has been minimized.

Copy link
Member Author

ddebroy commented Jan 12, 2019

@smarterclayton this is now ready ... PTAL for approval.

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Jan 14, 2019

/approve

based on sig-arch approval and no obvious name suggestions

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 14, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, smarterclayton

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

@ddebroy

This comment has been minimized.

Copy link
Member Author

ddebroy commented Jan 14, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 774fa8b into kubernetes:master Jan 15, 2019

18 checks passed

cla/linuxfoundation ddebroy authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@saad-ali
Copy link
Member

saad-ali left a comment

Sorry for the late review. Mostly nits. Feel free to address in a follow up PR.

@davidz627 @ddebroy

out-of-tree CSI components like external provisioner to facilitate migration of
code from Kubernetes In-tree plugin code to CSI plugin repositories.

Consumers can make use of this repository can make use of functions like

This comment has been minimized.

Copy link
@saad-ali

saad-ali Jan 15, 2019

Member

...can make use of this repository can make use...?

)

// GCEPD handles translation of PV spec from In-tree GCE PD to CSI GCE PD and vice versa
type GCEPD struct{}

This comment has been minimized.

Copy link
@saad-ali

saad-ali Jan 15, 2019

Member

Add compile time checks to ensure AWSEBS, GCEPD, etc. structs implement the InTreePlugin interface:

var _ InTreePlugin = GCEPD{}
)

// GCEPD handles translation of PV spec from In-tree GCE PD to CSI GCE PD and vice versa
type GCEPD struct{}

This comment has been minimized.

Copy link
@saad-ali

saad-ali Jan 15, 2019

Member

Make AWSEBS, GCEPD, etc. structs private (instead of public) and add a constructor methods that return the InTreePlugin interface so that consumers depend only on the interface:

func NewGCEPD(...) InTreePlugin {
  return &gcePD{...}
}

type gcePD struct{}

// GCEPD handles translation of PV spec from In-tree GCE PD to CSI GCE PD and vice versa
type GCEPD struct{}

This comment has been minimized.

Copy link
@saad-ali

saad-ali Jan 15, 2019

Member

Can we come up with more descriptive names then AWSEBS, GCEPD, etc.? Maybe gcePDCSIConverter or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.