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

CSI Cluster Registry and Node Info CRDs #67803

Merged
merged 6 commits into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/generated/openapi/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ openapi_library(
"k8s.io/client-go/pkg/apis/clientauthentication/v1alpha1",
"k8s.io/client-go/pkg/apis/clientauthentication/v1beta1",
"k8s.io/client-go/pkg/version",
"k8s.io/csi-api/pkg/apis/csi/v1alpha1",
"k8s.io/kube-aggregator/pkg/apis/apiregistration/v1",
"k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1",
"k8s.io/metrics/pkg/apis/custom_metrics/v1beta1",
Expand Down
1 change: 1 addition & 0 deletions staging/src/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ filegroup(
"//staging/src/k8s.io/code-generator/hack:all-srcs",
"//staging/src/k8s.io/code-generator/pkg/util:all-srcs",
"//staging/src/k8s.io/code-generator/third_party/forked/golang/reflect:all-srcs",
"//staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1:all-srcs",
"//staging/src/k8s.io/csi-api/pkg/client/listers/csi/v1alpha1:all-srcs",
"//staging/src/k8s.io/kube-aggregator:all-srcs",
"//staging/src/k8s.io/kube-proxy/config/v1alpha1:all-srcs",
Expand Down
32 changes: 32 additions & 0 deletions staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "go_default_library",
srcs = [
"doc.go",
"register.go",
"types.go",
],
importmap = "k8s.io/kubernetes/vendor/k8s.io/csi-api/pkg/apis/csi/v1alpha1",
importpath = "k8s.io/csi-api/pkg/apis/csi/v1alpha1",
visibility = ["//visibility:public"],
deps = [
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)
20 changes: 20 additions & 0 deletions staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
Copyright 2018 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

package v1alpha1 // import "k8s.io/csi-api/pkg/apis/csi/v1alpha1"
50 changes: 50 additions & 0 deletions staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/register.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2018 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
)

// GroupName is the group name use in this package
const GroupName = "csi.storage.k8s.io"

// SchemeGroupVersion is group version used to register these objects
var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha1"}

// Resource takes an unqualified resource and returns a Group qualified GroupResource
func Resource(resource string) schema.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}

var (
SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)
AddToScheme = SchemeBuilder.AddToScheme
)

// Adds the list of known types to the given scheme.
func addKnownTypes(scheme *runtime.Scheme) error {
scheme.AddKnownTypes(SchemeGroupVersion,
&CSIDriver{},
&CSIDriverList{},
)

metav1.AddToGroupVersion(scheme, SchemeGroupVersion)
return nil
}
76 changes: 76 additions & 0 deletions staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
Copyright 2018 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

// +genclient
// +genclient:nonNamespaced
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// CSIDriver captures information about a Container Storage Interface (CSI)
// volume driver deployed on the cluster.
// CSIDriver objects are non-namespaced.
type CSIDriver struct {
metav1.TypeMeta `json:",inline"`

// Standard object metadata.
// metadata.Name indicates the name of the CSI driver that this object
// refers to; it MUST be the same name returned by the CSI GetPluginName()
// call for that driver.
// More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata
metav1.ObjectMeta `json:"metadata,omitempty"`

// Specification of the CSI Driver.
Spec CSIDriverSpec `json:"spec"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// CSIDriverList is a collection of CSIDriver objects.
type CSIDriverList struct {
metav1.TypeMeta `json:",inline"`

// Standard list metadata
// More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata
// +optional
metav1.ListMeta `json:"metadata,omitempty"`

// Items is the list of CSIDriver
Items []CSIDriver `json:"items"`
}

// CSIDriverSpec is the specification of a CSIDriver.
type CSIDriverSpec struct {
// 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 false -- meaning attach will not 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
Copy link
Member

Choose a reason for hiding this comment

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

Why would we not pass this always?

Copy link
Member Author

Choose a reason for hiding this comment

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

"Backwards compatibility" and "ability to detect false vs user did not specify" are the usual reasons for making boolean fields a pointer. Neither of those are an issue here, so consistency? The AttachRequired field above needs to be able to differentiate between false vs user did not specify, so it has to be a pointer. And all future boolean fields added here will have to be pointers for backwards compat. So consistency with the other field here and future fields added.

Copy link
Member

Choose a reason for hiding this comment

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

My question was why this is an option at all - under what circumstances would you not want that info? I mean, some might not need it, but why not pass it anyway?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 to @msau42. These parameters are not part of the CSI spec. Most plugins won't use them. The plugins that need them must opt-in to this behavior.

Copy link
Member

Choose a reason for hiding this comment

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

blech, can we fix CSI to be less brittle?

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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).

Copy link
Member Author

Choose a reason for hiding this comment

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

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

That's a good idea. Changed this from a boolean to a version counter: PodInfoOnMountVersion *string with the comment documentation capturing accepted version value.

// will not be passed on mount.
// +optional
PodInfoRequiredOnMount *bool `json:"podInfoRequiredOnMount"`
}