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

📝 Mount API POC #3232

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 8 additions & 1 deletion sdk/apis/tenancy/v1alpha1/types_workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,18 @@ type WorkspaceSpec struct {
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.path) || !has(self.path) || self.path == oldSelf.path",message="path is immutable"
Type WorkspaceTypeReference `json:"type,omitempty"`

// MountName is the name of the mount that this workspace is mounted under.
//
// +optional
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.name) || !has(self.name) || self.name == oldSelf.name",message="name is immutable"
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.namespace) || !has(self.namespace) || self.namespace == oldSelf.namespace",message="namespace is immutable"
Mount *metav1.ObjectMeta `json:"mount,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this need some higher-level CEL rule to make it mutual exclusive and bound to type?


// location constraints where this workspace can be scheduled to.
//
// If the no location is specified, an arbitrary location is chosen.
//
// +optional
// +optionalz``
Location *WorkspaceLocation `json:"location,omitempty"`

// cluster is the name of the logical cluster this workspace is stored under.
Expand Down
6 changes: 6 additions & 0 deletions sdk/apis/tenancy/v1alpha1/types_workspacetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ type WorkspaceTypeSpec struct {
//
// +optional
DefaultAPIBindings []APIExportReference `json:"defaultAPIBindings,omitempty"`

// Mount is the mount configuration for the workspace type. Single type can have only one mount kind.
// If set, the workspace will be backed by a mount.
//
// +optional
Mount *metav1.TypeMeta `json:"mount,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

how is this related to workspace types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be something like kind: mounts.contrib.kcp.io so every workspace initiated via this type just needs to carry name only for the kind. basically bounts workspaceType to mount implementation kind. Still thinking if this is a good idea.

But if we keep Kind of mount object inside workspace spec, workspace Type is not even used.

maybe at the workspace level:
Type WorkspaceTypeReference should become with Kind: Mount,LogicalCluster ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

something like:

spec:
  URL: https://192.168.3.55:6443/clusters/root:consumer
  cluster: ofebwh0c93o6swd3
  type:
    name: organization
    path: root

Would become:

spec:
  URL: https://192.168.3.55:6443/clusters/root:consumer
  cluster: ofebwh0c93o6swd3
  type:
    kind: WorkspaceType
    name: organization
    path: root

and

spec:
  URL: https://192.168.3.55:6443/clusters/root:consumer
  cluster: ofebwh0c93o6swd3
  type:
    kind: Mount
    name: mountBob
    kind: mounts.contrib.kcp.io
    version: v1alpha1

Copy link
Member

Choose a reason for hiding this comment

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

In my mind type and mount are mutual exclusive, i.e. WorkspaceTypes are actually LogicalClusterTypes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So do you think its root level new field in Workspace? Any suggestions how this could be named considering type is taken? :) Or just mount:

}

// APIExportReference provides the fields necessary to resolve an APIExport.
Expand Down
Loading