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.

Sign up for GitHub

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

Jump to bottom

✨ Add pods resource to the default Kubernetes APIExport #2417

Merged
merged 6 commits into from
Feb 7, 2023
Merged

✨ Add pods resource to the default Kubernetes APIExport #2417

merged 6 commits into from
Feb 7, 2023

Conversation

jmprusi
Copy link
Member

@jmprusi jmprusi commented Nov 28, 2022 •
edited

Summary

For supporting pod subresources without extra manual steps, we need some additional changes:

  • Adds PODs by default to the kubernetes APIexport.
  • Don't expose pods via the syncer virtual workspace.
  • Clean up unnecessary steps from other tests because the Pods and permissions are correctly set.
  • Changes to the kcp sync command to add permissions downstream for pod subresources.

Related issue(s)

Fixes #2415

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 28, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jmprusi jmprusi changed the title ✨ Bootstraps the POD compute resource if the TunnelSyncer feature gate is enabled. ✨ Adds the Pod resource to the default Kubernetes apiexport. Nov 28, 2022
@davidfestal davidfestal changed the title ✨ Adds the Pod resource to the default Kubernetes apiexport. WIP ✨ Adds the Pod resource to the default Kubernetes apiexport. Nov 29, 2022
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch 2 times, most recently from 9a14a61 to 0ec8a68 Compare November 29, 2022 11:17
@jmprusi jmprusi changed the title WIP ✨ Adds the Pod resource to the default Kubernetes apiexport. ✨ Adds the Pod resource to the default Kubernetes apiexport. Nov 29, 2022
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch 2 times, most recently from 2a9c010 to 7f46489 Compare December 1, 2022 11:05
@jmprusi
Copy link
Member Author

jmprusi commented Dec 1, 2022

/retest

@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch 3 times, most recently from 6429938 to 9f1dd9c Compare December 1, 2022 15:42
@davidfestal davidfestal added the area/transparent-multi-cluster Related to scheduling of workloads into pclusters. label Dec 2, 2022
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch from 9f1dd9c to b0374f9 Compare December 2, 2022 11:15
@jmprusi jmprusi marked this pull request as ready for review December 2, 2022 11:33
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2022
@jmprusi
Copy link
Member Author

jmprusi commented Dec 2, 2022

/retitle Adds pod resource to the default Kubernetes apiexport.

@openshift-ci openshift-ci bot changed the title ✨ Adds the Pod resource to the default Kubernetes apiexport. Adds pod resource to the default Kubernetes apiexport. Dec 2, 2022
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch from b0374f9 to 348a8b8 Compare December 2, 2022 11:38
@jmprusi jmprusi changed the title Adds pod resource to the default Kubernetes apiexport. ✨ Adds pod resource to the default Kubernetes apiexport. Dec 2, 2022
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch 2 times, most recently from cc3a9d1 to 509f0db Compare December 2, 2022 17:34
@ncdc ncdc changed the title ✨ Adds pod resource to the default Kubernetes apiexport. ✨ Add pod resource to the default Kubernetes apiexport. Dec 2, 2022
@ncdc ncdc changed the title ✨ Add pod resource to the default Kubernetes apiexport. ✨ Add pod resource to the default Kubernetes apiexport Dec 2, 2022
@ncdc ncdc changed the title ✨ Add pod resource to the default Kubernetes apiexport ✨ Add pods resource to the default Kubernetes APIExport Dec 2, 2022
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch from 509f0db to f6fb1b2 Compare December 2, 2022 19:11
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch 9 times, most recently from 5519167 to 94e132f Compare February 2, 2023 18:28
@jmprusi
Copy link
Member Author

jmprusi commented Feb 2, 2023

The commit that caused the hold was dropped, unholding.
/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 2, 2023
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch from 94e132f to c5f5e44 Compare February 3, 2023 10:25
@jmprusi jmprusi force-pushed the 2415-when-the-kcpsyncertunnel-feature-gate-is-enabled-pods-should-be-added-to-the-compute-workspaces-automatically branch from c5f5e44 to e20bc8f Compare February 6, 2023 15:37
@davidfestal
Copy link
Member

/lgtm

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

@sttts Do you want to give a last look and approve ?

@jmprusi jmprusi requested a review from sttts February 6, 2023 16:10
@sttts
Copy link
Member

sttts commented Feb 7, 2023

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts

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 7, 2023
@davidfestal
Copy link
Member

/test e2e-sharded

flake ##2714

@openshift-merge-robot openshift-merge-robot merged commit 3a58b1c into kcp-dev:main Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidfestal davidfestal davidfestal left review comments

@shawn-hurley shawn-hurley Awaiting requested review from shawn-hurley

@ncdc ncdc Awaiting requested review from ncdc

@sttts sttts Awaiting requested review from sttts

Assignees

@davidfestal davidfestal

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/transparent-multi-cluster Related to scheduling of workloads into pclusters. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When the KCPSyncerTunnel feature gate is enabled, PODs should be added to the compute workspaces automatically
5 participants
@jmprusi @davidfestal @sttts @ncdc @openshift-merge-robot