Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Create HierarchicalNamespace CRD #448

Merged

Conversation

yiqigao217
Copy link
Contributor

@yiqigao217 yiqigao217 commented Feb 24, 2020

This CRD is created as the UX of the HNC self-service namespace. See
design at http://bit.ly/hnc-self-serve-ux.

Tested on a GKE cluster by "kubectl apply -f" the generated CRD yaml
file. The CRD was created with success. I also added a sample hns.yaml
and ran "kubectl create -f", the hns object was created and showed up
when I ran "kubectl get hns".

The possibility of "kubectl create hns" is still under investigation,
since the "kubectl create" only supports a selected list of built-in
resource types (even "kubectl create pod" is not supported). We will see
if "kubectl create" a CR object is possible.

Part of #457

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 24, 2020
@yiqigao217

This comment has been minimized.

@yiqigao217

This comment has been minimized.

@yiqigao217 yiqigao217 force-pushed the self-service-namespace branch from 8a39dbb to 4b5ab13 Compare February 24, 2020 21:47
Copy link
Contributor

@adrianludwin adrianludwin left a comment

Choose a reason for hiding this comment

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

lgtm after one last change

incubator/hnc/api/v1alpha1/hierarchical_namespace.go Outdated Show resolved Hide resolved
@yiqigao217 yiqigao217 force-pushed the self-service-namespace branch 2 times, most recently from 253fb42 to 4b0bd4e Compare February 25, 2020 15:16
@adrianludwin
Copy link
Contributor

lgtm. Can you confirm that if you just say kubectl create hns bar -n foo (or kubectl create hierarchicalnamespace bar -n foo, then an empty HNS object called "bar" gets created in namespace "foo"?

@adrianludwin
Copy link
Contributor

/assign @rjbez17

@adrianludwin
Copy link
Contributor

/hold
/lgtm
/approve

For Ryan to review.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 25, 2020
@yiqigao217

This comment has been minimized.

This CRD is created as the UX of the HNC self-service namespace. See
design at http://bit.ly/hnc-self-serve-ux.

Tested on a GKE cluster by "kubectl apply -f" the generated CRD yaml
file. The CRD was created with success. I also added a sample hns.yaml
and ran "kubectl create -f", the hns object was created and showed up
when I ran "kubectl get hns".

The possibility of "kubectl create hns" is still under investigation,
since the "kubectl create" only supports a selected list of built-in
resource types (even "kubectl create pod" is not supported). We will see
if "kubectl create" a CR object is possible.
@yiqigao217 yiqigao217 force-pushed the self-service-namespace branch from 4b0bd4e to 1198263 Compare February 25, 2020 19:57
@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 25, 2020
@yiqigao217
Copy link
Contributor Author

I added a sample hns.yaml for kubectl to apply to create an hns object and updated the commit body with the explanation that all commands kubectl apply -f crd.yaml, kubectl create -f hns.yaml and kubectl get hns worked while kubectl create hns not.

It's a kubectl related issue that CR objects can be created by kubectl create -f but not directly by kubectl create, which is out of the scope of this PR.

@yiqigao217
Copy link
Contributor Author

/assign @sophieliu15

@k8s-ci-robot
Copy link
Contributor

@yiqigao217: GitHub didn't allow me to assign the following users: sophieliu15.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @sophieliu15

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.

@adrianludwin
Copy link
Contributor

Ok, pity that we can't do k create hns, we'll just have to go with k hns create for this one case.

/shrug

I'm ok with this one @rjbez17 is. I think he's OOO today so perhaps he'll reply tomorrow, if not lmk and I'll approve this myself.

@k8s-ci-robot k8s-ci-robot added the ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯ label Feb 25, 2020
@rjbez17
Copy link

rjbez17 commented Feb 25, 2020

/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 25, 2020
@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 25, 2020
@k8s-ci-robot k8s-ci-robot merged commit 034685a into kubernetes-retired:master Feb 25, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianludwin, rjbez17, yiqigao217

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:
  • OWNERS [adrianludwin,rjbez17]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants