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

⚠️ ClusterWorkspace => LogicalCluster refactor #2510

Conversation

sttts
Copy link
Member

@sttts sttts commented Dec 19, 2022

High-level overview: https://hackmd.io/Qb3HX2XGRV63Rt-83dZXNg

TODOs:

  • Rename ThisWorkspace to LogicalCluster, group core.kcp.dev.
  • Rename ClusterWorkspaceShard to Shard, group core.kcp.dev.
  • Rename ClusterWorkspaceType to WorkspaceType.
  • Rename tenancy.kcp.dev/path to kcp.dev/paths, and move into core package.
  • create ThisWorkspace in some logical cluster that is NOT the workspace path: base36(sha224(random)) and name with cluster name.
  • Rename LogicalCluster objects to "cluster"

Revisit:

Follow-ups:

sttts and others added 30 commits December 19, 2022 11:40
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 20, 2022

[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

@ncdc ncdc added this to the v0.11 milestone Dec 20, 2022
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2022
@sttts
Copy link
Member Author

sttts commented Dec 20, 2022

/retest

@ncdc
Copy link
Member

ncdc commented Dec 20, 2022

Hit #2501

@ncdc
Copy link
Member

ncdc commented Dec 20, 2022

=== FAIL: test/e2e/apibinding TestAPIBindingAPIExportReferenceImmutability (49.16s)
    kcp.go:127: shared kcp server will target configuration "/go/src/github.com/kcp-dev/kcp/.kcp/admin.kubeconfig"
    assertions.go:1691: Waiting for condition, but got: workspace phase is Initializing, not Ready
    workspaces.go:142: Created root:organization workspace root:e2e-workspace-7mp22
    assertions.go:1691: Waiting for condition, but got: workspace phase is Initializing, not Ready
    workspaces.go:142: Created root:universal workspace 57vxee58had43wkn:service-provider-1
    assertions.go:1691: Waiting for condition, but got: workspace phase is Initializing, not Ready
    workspaces.go:142: Created root:universal workspace 57vxee58had43wkn:consumer-1-bound-against-1
    apibinding_test.go:74: Create an APIExport today-cowboys in "k4gbt3w1old718qw"
    apibinding_test.go:97: Create an APIBinding in "8pxokwmrdqcjioq9" that points to the today-cowboys export from "k4gbt3w1old718qw"
    apibinding_test.go:125: 
        	Error Trace:	apibinding_test.go:125
        	Error:      	Error "apibindings.apis.kcp.dev \"cowboys\" is forbidden: unable to update APIBinding: no permission to bind to export k4gbt3w1old718qw:other-export" does not contain "APIExport reference must not be changed"
        	Test:       	TestAPIBindingAPIExportReferenceImmutability

We swallow the errors that would indicate why 😦

@ncdc
Copy link
Member

ncdc commented Dec 20, 2022

/retest

@ncdc
Copy link
Member

ncdc commented Dec 20, 2022

=== CONT  TestPlacementUpdate
    syncer.go:594: checking secret kcp-syncer-synctarget-634511-m74yaiva/default-token-m5fjt for annotation kubernetes.io/service-account.name=kcp-syncer-synctarget-634511-m74yaiva
    syncer.go:594: checking secret kcp-syncer-synctarget-634511-m74yaiva/kcp-syncer-synctarget-634511-m74yaiva for annotation kubernetes.io/service-account.name=kcp-syncer-synctarget-634511-m74yaiva
    syncer.go:594: checking secret kcp-syncer-synctarget-634511-m74yaiva/kcp-syncer-synctarget-634511-m74yaiva-token for annotation kubernetes.io/service-account.name=kcp-syncer-synctarget-634511-m74yaiva
I1220 19:42:33.229391   28635 syncer.go:79] "starting syncer" syncTarget.workspace="211wd4ehtyqtyd6q" syncTarget.name="synctarget-634511"
I1220 19:42:33.231408   28635 syncer.go:106] "attempting to retrieve the Syncer virtual workspace URL" syncTarget.workspace="211wd4ehtyqtyd6q" syncTarget.name="synctarget-634511"
    syncer.go:372: 
        	Error Trace:	syncer.go:372
        	            				placement_scheduler_test.go:70
        	Error:      	Received unexpected error:
        	            	the server has asked for the client to provide credentials
        	Test:       	TestPlacementUpdate
        	Messages:   	syncer failed to start
    syncer.go:202: Collecting imported resource info: /logs/artifacts/TestPlacementUpdate/3876109408/artifacts
I1220 19:42:33.311067   28635 reflector.go:255] Listing and watching *unstructured.Unstructured from k8s.io/client-go@v0.0.0-20221215125340-abcb3d0c8338/tools/cache/reflector.go:167
    syncer.go:211: Error gathering /v1, Resource=services: the server could not find the requested resource
    syncer.go:211: Error gathering networking.k8s.io/v1, Resource=ingresses: the server could not find the requested resource
    syncer.go:211: Error gathering apps/v1, Resource=deployments: the server could not find the requested resource
--- FAIL: TestPlacementUpdate (83.76s)

https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/kcp-dev_kcp/2510/pull-ci-kcp-dev-kcp-main-e2e-shared/1605285968393277440

@ncdc
Copy link
Member

ncdc commented Dec 20, 2022

/retest

1 similar comment
@sttts
Copy link
Member Author

sttts commented Dec 20, 2022

/retest

@openshift-merge-robot openshift-merge-robot merged commit c67d504 into kcp-dev:main Dec 20, 2022
@sttts
Copy link
Member Author

sttts commented Dec 20, 2022

🎉

@sttts
Copy link
Member Author

sttts commented Jan 5, 2023

@ncdc Re: Move deletion of logical cluster rbac data to owner refs/gc

This would require blocking the LogicalCluster deletion as otherwise no gc is running for the logical cluster. Doesn't make the logic simpler, does it?

@ncdc
Copy link
Member

ncdc commented Jan 5, 2023

@sttts I thought there was maybe a TODO somewhere for that, but now I'm not seeing it. If it's not worth it, we can cross it off the list

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. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants