CSI Cluster Registry and Node Info CRDs #67803

saad-ali · 2018-08-24T08:30:58Z

What this PR does / why we need it:
Introduces the new CSIDriver and CSINodeInfo API Object as proposed in kubernetes/community#2514 and kubernetes/community#2034

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#594

Special notes for your reviewer:
Per the discussion in https://groups.google.com/d/msg/kubernetes-sig-storage-wg-csi/x5CchIP9qiI/D_TyOrn2CwAJ the API is being added to the staging directory of the kubernetes/kubernetes repo because the consumers will be attach/detach controller and possibly kubelet, but it will be installed as a CRD (because we want to move in the direction where the API server is Kubernetes agnostic, and all Kubernetes specific types are installed).

Release note:

Introduce CSI Cluster Registration mechanism to ease CSI plugin discovery and allow CSI drivers to customize Kubernetes' interaction with them.

CC @jsafrane

verult · 2018-08-25T02:30:52Z

/cc

saad-ali · 2018-08-26T23:04:50Z

/test pull-kubernetes-integration

saad-ali · 2018-08-27T08:52:12Z

pkg/features/kube_features.go

@@ -394,6 +400,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	MountContainers:                             {Default: false, PreRelease: utilfeature.Alpha},
 	VolumeScheduling:                            {Default: true, PreRelease: utilfeature.Beta},
 	CSIPersistentVolume:                         {Default: true, PreRelease: utilfeature.Beta},
+	CSICrdAutoInstall:                           {Default: true, PreRelease: utilfeature.Beta},


For testing only. Will flip to false and alpha before merge.

k8s-ci-robot · 2018-08-31T20:08:19Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~cmd/kube-controller-manager/OWNERS~~ [thockin]
~~cmd/kubelet/OWNERS~~ [thockin]
~~hack/OWNERS~~ [thockin]
~~pkg/OWNERS~~ [thockin]
~~staging/OWNERS~~ [thockin]
~~test/OWNERS~~ [saad-ali,thockin]
~~vendor/OWNERS~~ [thockin]

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 · 2018-08-31T20:25:21Z

@saad-ali: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-device-plugin-gpu	`fdeb895`	link	`/test pull-kubernetes-e2e-gce-device-plugin-gpu`

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.

lavalamp

here are some initial thoughts. Thank you for putting it as separate commits, that made it much easier to review.

lavalamp · 2018-08-31T20:28:23Z

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/doc.go

+
+// +k8s:deepcopy-gen=package,register
+// +groupName=csi.storage.k8s.io
+// +k8s:openapi-gen=true


Add a blank line to prevent this from being a godoc package comment

lavalamp · 2018-08-31T20:39:34Z

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go

+	// (like podName, podUID, etc.) during mount operations.
+	// If this is set to true, Kubelet will pass pod information as
+	// VolumeAttributes in the CSI NodePublishVolume() calls.
+	// If value is not specified, default is false -- meaning pod information


It sounds like this should be a version counter that controls the format of the data passed, not a true/false field?

lavalamp · 2018-08-31T20:41:58Z

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// CSINodeInfo holds information about all CSI drivers installed on a node.
+type CSINodeInfo struct {


Info is very vague. Is this spec (declaring which drivers should be running), status (reporting which drivers are running), both, or neither?

Who writes these objects? Who reads them? (answer in the comment)

lavalamp · 2018-08-31T20:42:40Z

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go

+
+// CSIDriverInfo contains information about one CSI driver installed on a node.
+type CSIDriverInfo struct {
+	// Driver is the name of the CSI driver that this object refers to.


driver

(use json capitalization)

lavalamp · 2018-08-31T20:43:46Z

staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go

+	// that driver.
+	Driver string `json:"driver"`
+
+	// ID of the node from the driver point of view.


Explain this more. Why is it different from e.g. the node's UID?

Will clarify.

lavalamp · 2018-08-31T20:59:50Z

pkg/controller/volume/attachdetach/attach_detach_controller.go

+	if err != nil && !apierrors.IsAlreadyExists(err) {
+		glog.Errorf("failed to create CSIDrivers CRD: %#v, err: %#v",
+			res, err)
+	} else if apierrors.IsAlreadyExists(err) {


is it safe to pass a nil error to this function?

Good point. Cleaned up the block to prevent that.

lavalamp · 2018-08-31T21:02:28Z

pkg/controller/volume/attachdetach/attach_detach_controller.go

+			res)
+	}
+
+	return nil


How well will the rest of the system cope if only one of these is installed (say the other one has a transient error)?

Different parts of the volume plugin provisioning/attach/mount process look at CRs for one of these CRDs. If one of the CRDs fails to create, it would just change where in the provision/attach/mount process subsequent failure will happen.

Since it is better to fail fast, I changed the errors here to return error instead of silently log.

lavalamp · 2018-08-31T21:04:09Z

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -649,6 +667,65 @@ func (adc *attachDetachController) processVolumesInUse(
 	}
 }

+// installCRDs creates the specified CustomResourceDefinition for the CSIDrivers object.
+func (adc *attachDetachController) installCRDs() error {


I am a little concerned about the implications of controller-manager doing schema changes when it starts up. Not claiming it's definitely problematic, just still thinking about it.

Ya, we're doing this for alpha because it's the path of least resistance. For beta, I was talking to @timstclair and we were thinking about having a new/dedicated controller that is responsible for installing CRDs (and reinstalling them if they are uninstalled, e.g. like add on manager). Can revisit that for beta.

Should we just use addons for alpha? I drafted those changes here: tallclair@a58008e

we're doing this for alpha because it's the path of least resistance

That attitude is concerning. Constructing APIs via CRD should not lower our standards for design or make us more willing to ship things we haven't considered the implications of.

lavalamp · 2018-08-31T21:05:37Z

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -649,6 +667,65 @@ func (adc *attachDetachController) processVolumesInUse(
 	}
 }

+// installCRDs creates the specified CustomResourceDefinition for the CSIDrivers object.
+func (adc *attachDetachController) installCRDs() error {
+	crd := &apiextensionsv1beta1.CustomResourceDefinition{


I think the definition files should be stored in yaml format in a file. I don't think it's a good idea to construct them at runtime, it's too easy to make a change to the code and not realize you're introducing a compatibility problem.

Noted. When we move this to a more permanent solution in beta (see #67803 (comment)), will consider this as well.

I think the definition files should be stored in yaml format in a file. I don't think it's a good idea to construct them at runtime, it's too easy to make a change to the code and not realize you're introducing a compatibility problem.

Why was this comment not considered necessary to address? I am somewhat dismayed by the attitude (echoed in the follow up PR) that the issues raised here don't matter for alpha.

lavalamp · 2018-08-31T21:06:33Z

pkg/controller/volume/attachdetach/attach_detach_controller.go

+				Plural: csiapiv1alpha1.CsiDriverResourcePlural,
+				Kind:   reflect.TypeOf(csiapiv1alpha1.CSIDriver{}).Name(),
+			},
+		},


Why not use the validation property? I think you should. I think there is a way to construct it from the types.go file you have.

Oh nice, I didn't realize validation was already supported. Looks like auto generation of OpenAPIV3Schema from types.go is still in the works kubernetes/kube-openapi#37. Before we move to beta, hopefully that is ready and we can use it. If not, we'll just construct the Validation schema manually then.

thockin · 2018-08-31T22:03:48Z

The fact that CSI plugins must fail if they get info they don't understand seems weird. Can we explore that?

…

On Fri, Aug 31, 2018 at 2:38 PM Saad Ali ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go <#67803 (comment)> : > + Driver string `json:"driver"` + + // Indicates this CSI volume driver requires an attach operation (because it + // implements the CSI ControllerPublishVolume() method), and that Kubernetes + // should call attach and wait for any attach operation to complete before + // proceeding to mounting. + // If value is not specified, default is true -- meaning attach will be + // called. + // +optional + AttachRequired *bool `json:"attachRequired"` + + // Indicates this CSI volume driver requires additional pod information + // (like podName, podUID, etc.) during mount operations. + // If this is set to true, Kubelet will pass pod information as + // VolumeAttributes in the CSI NodePublishVolume() calls. + // If value is not specified, default is false -- meaning pod information blech, can we fix CSI to be less brittle? We looked in to adding workload info into CSI. The challange is that CSI is agnostic to the CO (has to work with Kubernetes, Mesos, and others). So Kubernetes specific pod information can't be encoded in to the spec. But we'd still like to be able to have CSI drivers that want this Kube specific info to be able to receive it. And this is a relatively clean way to do so (leaves CSI spec agnostic, and allows individual drivers to choose not to be). — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#67803 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVBDyVEdh_FQJ1TkochxiQRUrPjjHks5uWazGgaJpZM4WK9To> .

saad-ali · 2018-08-31T23:05:13Z

Currently all plugins are supposed to validate every volume attribute and error on attributes they don't recognize. This could break plugins when we upgrade Kubernetes independently of the plugin

The fact that CSI plugins must fail if they get info they don't understand seems weird. Can we explore that?

To be clear, the CSI spec does not say that a volume plugin must (or should) fail if it sees volume attributes it does not recognize. It leaves validation of volume attributes up to the volume plugin to determine. For backwards compat, a well behaved plugin should validate the parameters it cares about, and ignore the ones it doesn't understand. But, like storageClass parameters, ultimately it is up to the driver to determine what type of validation it will do.

thockin · 2018-08-31T23:13:12Z

Would you say that a driver which explodes when given a parameter it doesn't know is well-behaved or mis-behaved?

…

On Fri, Aug 31, 2018 at 4:05 PM Saad Ali ***@***.***> wrote: Currently all plugins are supposed to validate every volume attribute and error on attributes they don't recognize. This could break plugins when we upgrade Kubernetes independently of the plugin The fact that CSI plugins must fail if they get info they don't understand seems weird. Can we explore that? To be clear, the CSI spec does not say that a volume plugin must (or should) fail if it sees volume attributes it does not recognize. It leaves validation of volume attributes up to the volume plugin to determine. For backwards compat, a well behaved plugin should validate the parameters it cares about, and ignore the ones it doesn't understand. But, like storageClass parameters, ultimately it is up to the driver to determine what type of validation it will do. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

msau42 · 2018-08-31T23:18:00Z

Should a driver that doesn't understand a parameter just blindly ignore it?

thockin · 2018-08-31T23:22:56Z

That's the general behavior of most things, isn't it? A C function that doesn't know a global variable ignores it. A shell program that doesn't know an env var ignores it. Do you have a "problem vector" in mind?

…

On Fri, Aug 31, 2018 at 4:18 PM Michelle Au ***@***.***> wrote: Should a driver that doesn't understand a parameter just blindly ignore it? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#67803 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVGEAW8XKAXWGtBQtIK8VPKyWkab6ks5uWcRFgaJpZM4WK9To> .

msau42 · 2018-08-31T23:44:14Z

Do you have a "problem vector" in mind?

I'm mostly thinking about how StorageClass parameters are validated today for in-tree plugins. They fail if an unrecognized parameter is passed in to prevent an incorrect volume to be provisioned. For example, a user could mispell the parameter for "encryptionMode". The plugin could ignore it and provision an unencrypted volume when the user expected an encrypted one.

k8s-github-robot · 2018-08-31T23:46:39Z

Automatic merge from submit-queue (batch tested with PRs 64283, 67910, 67803, 68100). 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.

saad-ali · 2018-09-01T00:42:51Z

I'm mostly thinking about how StorageClass parameters are validated today for in-tree plugins. They fail if an unrecognized parameter is passed in to prevent an incorrect volume to be provisioned. For example, a user could mispell the parameter for "encryptionMode". The plugin could ignore it and provision an unencrypted volume when the user expected an encrypted one.

Good point. This is a good reason for CSI to leave it up to the driver to determine what validation it wants to do (it can optimize for backwards compat or correctness) .

saad-ali · 2018-09-01T00:57:19Z

This PR merged before I could address @lavalamp's comments. Opened up follow up PR #68159 to address his comments.

@lavalamp

Automatic merge from submit-queue (batch tested with PRs 66840, 68159). 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. CSI Cluster Registry and Node Info CRDs Improvements **What this PR does / why we need it**: #67803 merged before I could address @lavalamp's feedback. This PR addresses his feedback **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Follow up on PR #67803 **Special notes for your reviewer**: **Release note**: ```release-note ``` /assign @lavalamp /assign @thockin CC @jsafrane @vladimirvivien @verult @gnufied @childsb

@lavalamp

Automatic merge from submit-queue (batch tested with PRs 66840, 68159). 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. CSI Cluster Registry and Node Info CRDs Improvements **What this PR does / why we need it**: kubernetes/kubernetes#67803 merged before I could address @lavalamp's feedback. This PR addresses his feedback **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Follow up on PR kubernetes/kubernetes#67803 **Special notes for your reviewer**: **Release note**: ```release-note ``` /assign @lavalamp /assign @thockin CC @jsafrane @vladimirvivien @verult @gnufied @childsb Kubernetes-commit: 416f63c050d01fe831d532d89e29d01093d9c127

@saad-ali

Automatic merge from submit-queue (batch tested with PRs 68161, 68023, 67909, 67955, 67731). 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. CSI: skip attach for non-attachable drivers **What this PR does / why we need it**: This is implementation of kubernetes/community#2523. CSI volumes that don't need attach/detach now don't need external attacher running. WIP: * contains #67803 to get CSIDriver API. Ignore the first commit. * ~~missing e2e test~~ /sig storage cc: @saad-ali @vladimirvivien @verult @msau42 @gnufied @davidz627 **Release note**: ```release-note CSI volume plugin does not need external attacher for non-attachable CSI volumes. ```

@saad-ali

Automatic merge from submit-queue (batch tested with PRs 68171, 67945, 68233). 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. CSI pod information in NodePublish **What this PR does / why we need it**: This is implementation of kubernetes/community#2439. It brings CSI closer to Flex volume features, CSI driver can (optionally) get name/namespace/UID/ServiceAccount of the pod that requires the CSI volume mounted. This allows CSI drivers to either do AAA before the volume is mounted or tailor content of volume to the pod. Work in progress: * contains #67803 to get `CSIDriver` API. Ignore the first commit. Related to #64984 #66362 (fixes only part of these issues) /sig storage cc: @saad-ali @vladimirvivien @verult @msau42 @gnufied **Release note**: ```release-note CSI NodePublish call can optionally contain information about the pod that requested the CSI volume. ```

@lavalamp

Automatic merge from submit-queue (batch tested with PRs 66840, 68159). 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. CSI Cluster Registry and Node Info CRDs Improvements **What this PR does / why we need it**: kubernetes/kubernetes#67803 merged before I could address @lavalamp's feedback. This PR addresses his feedback **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Follow up on PR kubernetes/kubernetes#67803 **Special notes for your reviewer**: **Release note**: ```release-note ``` /assign @lavalamp /assign @thockin CC @jsafrane @vladimirvivien @verult @gnufied @childsb Kubernetes-commit: 416f63c050d01fe831d532d89e29d01093d9c127

@lavalamp

Automatic merge from submit-queue (batch tested with PRs 66840, 68159). 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. CSI Cluster Registry and Node Info CRDs Improvements **What this PR does / why we need it**: kubernetes/kubernetes#67803 merged before I could address @lavalamp's feedback. This PR addresses his feedback **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Follow up on PR kubernetes/kubernetes#67803 **Special notes for your reviewer**: **Release note**: ```release-note ``` /assign @lavalamp /assign @thockin CC @jsafrane @vladimirvivien @verult @gnufied @childsb Kubernetes-commit: 416f63c050d01fe831d532d89e29d01093d9c127

smarterclayton · 2018-10-16T16:15:30Z

cmd/kube-controller-manager/app/core.go

@@ -192,9 +193,14 @@ func startAttachDetachController(ctx ControllerContext) (http.Handler, bool, err
 	if ctx.ComponentConfig.AttachDetachController.ReconcilerSyncLoopPeriod.Duration < time.Second {
 		return nil, true, fmt.Errorf("Duration time must be greater than one second as set via command line option reconcile-sync-loop-period.")
 	}
+	csiClientConfig := ctx.ClientBuilder.ConfigOrDie("attachdetach-controller")
+	// csiClient works with CRDs that support json only
+	csiClientConfig.ContentType = "application/json"


This shouldn't have been necessary. Clients should have priority order content accept types which should result in json being served. What happened when you didn't set this?

@liggitt wouldn't have expected this to show up in code using CRD

looks like

kubernetes/cmd/kube-controller-manager/app/options/options.go

Line 412 in 95c99eb

kubeconfig.ContentConfig.ContentType = s.Generic.ClientConnection.ContentType

defaulted at

kubernetes/staging/src/k8s.io/apimachinery/pkg/apis/config/v1alpha1/defaults.go

Lines 29 to 31 in 95c99eb

if len(obj.ContentType) == 0 {

obj.ContentType = "application/vnd.kubernetes.protobuf"

}

I'm not seeing a fallback specified

What happened when you didn't set this?

Watch() did not work. It received protobuf and failed to parse it.

is this related to #62175?

k8s-ci-robot requested review from brendandburns and caesarxuchao August 24, 2018 08:31

k8s-ci-robot added area/kubelet sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Aug 24, 2018

saad-ali mentioned this pull request Aug 24, 2018

[WIP] CSI Cluster Registry #67554

Closed

saad-ali force-pushed the csiClusterReg3 branch 2 times, most recently from b1667ef to db1c227 Compare August 24, 2018 19:42

k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Aug 24, 2018

k8s-ci-robot requested a review from verult August 25, 2018 02:30

verult mentioned this pull request Aug 25, 2018

CSI Node info registration in kubelet #67684

Merged

saad-ali force-pushed the csiClusterReg3 branch 2 times, most recently from 2cb115d to e024408 Compare August 26, 2018 21:48

saad-ali force-pushed the csiClusterReg3 branch from e024408 to ec34338 Compare August 27, 2018 08:46

saad-ali commented Aug 27, 2018

View reviewed changes

saad-ali changed the title ~~[WIP] CSI Cluster Registry and Node Info CRDs~~ CSI Cluster Registry and Node Info CRDs Aug 27, 2018

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 Aug 27, 2018

saad-ali requested review from thockin and jsafrane August 27, 2018 08:52

saad-ali added this to the v1.12 milestone Aug 27, 2018

saad-ali added the status/approved-for-milestone label Aug 27, 2018

saad-ali force-pushed the csiClusterReg3 branch 2 times, most recently from 541a23a to d32491f Compare August 27, 2018 15:55

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2018

lavalamp reviewed Aug 31, 2018

View reviewed changes

k8s-github-robot merged commit 85300f4 into kubernetes:master Aug 31, 2018

saad-ali mentioned this pull request Sep 1, 2018

CSI Cluster Registry and Node Info CRDs Improvements #68159

Merged

saad-ali mentioned this pull request Sep 1, 2018

CSI - API Changes for Pod Inline Volume Support #67452

Closed

This was referenced Sep 7, 2018

Attach Detach Controller CRD validations #68406

Closed

CRD creation RBAC policy for Attach Detach Controller #68519

Closed

smarterclayton reviewed Oct 16, 2018

View reviewed changes

This was referenced Oct 17, 2018

[client-go] Renegotiate Streaming Content-Type #62175

Closed

WIP - Fix watch event handling for CRDs #69987

Closed

kacole2 mentioned this pull request Oct 22, 2018

CSI Cluster Registration Mechanism kubernetes/enhancements#594

Closed

liggitt mentioned this pull request Nov 17, 2018

Make bootstrap client cert loading part of rotation #69890

Merged

msau42 mentioned this pull request Feb 26, 2019

Make CSINodeInfo and CSIDriver Core APIs #73883

Merged

	if len(obj.ContentType) == 0 {
	obj.ContentType = "application/vnd.kubernetes.protobuf"
	}

CSI Cluster Registry and Node Info CRDs #67803

CSI Cluster Registry and Node Info CRDs #67803

Conversation

saad-ali commented Aug 24, 2018

verult commented Aug 25, 2018

saad-ali commented Aug 26, 2018

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 31, 2018

k8s-ci-robot commented Aug 31, 2018 • edited

lavalamp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Aug 31, 2018 via email

saad-ali commented Aug 31, 2018

thockin commented Aug 31, 2018 via email

msau42 commented Aug 31, 2018

thockin commented Aug 31, 2018 via email

msau42 commented Aug 31, 2018 • edited

k8s-github-robot commented Aug 31, 2018

saad-ali commented Sep 1, 2018

saad-ali commented Sep 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 31, 2018 •

edited

msau42 commented Aug 31, 2018 •

edited