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

📖 APIExportEndpointSlice and Partition doc #2799

Merged
merged 2 commits into from
Feb 21, 2023

Conversation

fgiloux
Copy link
Contributor

@fgiloux fgiloux commented Feb 16, 2023

Summary

This PR adds some documentation for APIExportEndpointSlice, Partition and PartitionSet.
It also adds some information on PermissionClaim and changes wording from 'virtual workspace' to 'service endpoint'.

Related issue(s)

Fixes #2776

# Partition API

`Partitions` and `PartitionSets` build an API that allows service providers and cluster administrators to create shard Partitions. These partitions can then be leveraged:
- for the placement of the controllers of the service providers and other components. These APIs provide the information on shard topology required for the scheduling.
Copy link
Contributor

Choose a reason for hiding this comment

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

This says placement although does not relate to the Placement APIs? That could cause some confusion, might be worth to reword a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to deployment

path: root:wildwest:cowboys-service
name: cowboy
# optional
partition: us-west-1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
partition: us-west-1
partition: cloud-region-gcp-europe-xdfgs

Is this by name? Could we reference one of the example above?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could one reference a PartitionSet instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, only Partitions can be referenced. PartitionSets are just a convenient way of generating Partitions. If service providers know the Shards and their labels they could create Partitions directly without using a PartitionSet.
I can change the name.


A: Think of this virtual workspace as representing a wildcard listing across all workspaces. It doesn't make sense to
look at a specific namespace across all workspaces, so you have to list across all namespaces too.
A: Think of this endpoint as representing a wildcard listing across all workspaces. It doesn't make sense to look at a specific namespace across all workspaces, so you have to list across all namespaces too.
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why doesn't it make sense to only look at a single namespace (or a set of) across many workspaces?

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 is not from me so that I can only guess. One reason may be that the natural isolation level in kcp is the workspace rather than namespace.
I only intended to remove "virtual workspace" as we want to move away from this wording that has caused confusion.


Q: If I attempt to use an `APIExport` virtual workspace before there are any `APIBindings` I get the "Error from server
(NotFound): Unable to list ...: the server could not find the requested resource". Is this a bug?
Q: If I attempt to use an `APIExport` endpoint before there are any `APIBindings` I get the "Error from server (NotFound): Unable to list ...: the server could not find the requested resource". Is this a bug?

A: It is a bug. See <https://github.com/kcp-dev/kcp/issues/1183>
Copy link
Contributor

Choose a reason for hiding this comment

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

This issue was closed, is the above still valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is. The resolution was only partial, i.e. not working with shards, and not ported to the new API. The preferred approach for now would be to amend the virtual workspace so that an empty list is returned:
https://kubernetes.slack.com/archives/C021U8WSAFK/p1674303045697859?thread_ts=1674243613.104869&cid=C021U8WSAFK

Also add some information on PermissionClaim and change wording from 'virtual workspace' to 'service endpoint'.

Signed-off-by: Frederic Giloux <fgiloux@redhat.com>
…tion

Signed-off-by: Frederic Giloux <fgiloux@redhat.com>
@@ -293,13 +355,11 @@ Yay!

Q: Why is there a new `APIResourceSchema` resource type that appears to be very similar to `CustomResourceDefinition`?

A: FIXME
A: An APIResourceSchema defines a single custom API type. It is almost identical to a CRD, but creating an APIResourceSchema instance does not add a usable API to the server. By intentionally decoupling the schema definition from serving, API owners can be more explicit about API evolution.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 21, 2023
@openshift-merge-robot openshift-merge-robot merged commit 6434142 into kcp-dev:main Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add KCP user usage documentation for APIExportEndpointSlice, Partition
4 participants