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

✨ Support for pod logs and other subresources #2401

Merged

Conversation

jmprusi
Copy link
Member

@jmprusi jmprusi commented Nov 23, 2022

Summary

Adds support to kcp for POD subresources, adding the proper handler to use the Syncer Tunneler capabilities.

The Feature gate KCPSyncerTunnel should be enabled in KCP and all the desired Syncers:

--feature-gates KCPSyncerTunnel=true

There are still some missing parts to make this process "fully" automatic:

  • Upsyncer controller and POD upsyncing.
  • Automatically registering the POD resource.
  • Sync command generating the YAML with the propper permissions for POD/$subresources to the Syncer ClusterRole.

Steps to test this PR

Build the project and images:

make build build-kind-images-ko install

Run KCP:

./bin/kcp start --feature-gates KCPSyncerTunnel=true

Create your workspace:

KUBECONFIG=.kcp/admin.kubeconfig kubectl kcp ws create myorg --enter
KUBECONFIG=.kcp/admin.kubeconfig kubectl kcp ws create myworkspace --enter

Create your Synctarget, using the syncer imatge from the first step:

KUBECONFIG=.kcp/admin.kubeconfig kubectl kcp workload sync cluster1 --syncer-image=<syncer_image> -o syncer.yaml --resources=pods --feature-gates KCPSyncerTunnel=true

Now we need to modify the syncer.yaml file to add the missing ClusterRole permissions:

 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
   name: kcp-syncer-cluster2-33n2fkyd
 rules:
 - apiGroups:
   - ""
   resources:
   - namespaces
   verbs:
   - "create"
   - "list"
   - "watch"
   - "delete"
 - apiGroups:
   - "apiextensions.k8s.io"
   resources:
   - customresourcedefinitions
   verbs:
   - "get"
   - "watch"
   - "list"
 - apiGroups:
   - ""
   resources:
   - configmaps
   - secrets
   - services
   - pods
+  - pods/log
+  - pods/exec
   verbs:
   - "*"
 - apiGroups:
   - "apps"
   resources:
   - deployments
   verbs:
   - "*"
 - apiGroups:
   - "networking.k8s.io"
   resources:
   - ingresses
   verbs:
   - "*"
 ---

Deploy the syncer to out k8s cluster:

kubectl apply -f syncer.yaml

Let's bind the compute:

KUBECONFIG=.kcp/admin.kubeconfig kubectl kcp bind compute root:myorg:myworkspace

We should create a deployment now:

KUBECONFIG=.kcp/admin.kubeconfig kubectl create deployment kuard-tunneler --image gcr.io/kuar-demo/kuard-amd64:blue

Now the tricky part, we need to create a POD in KCP with the same name as the downstream one trough the Upsyncer.

Get the upsyncer URL:

KUBECONFIG=.kcp/admin.kubeconfig kubectl get synctarget cluster1 --output=jsonpath={.status.virtualWorkspaces[0].url} | sed 's/syncer/upsyncer/'

The URL should look like: https://172.22.10.60:6443/services/upsyncer/root:testing:testing/cluster1/ad76b43c-399b-40d7-b499-32a568e6861c

Get the synctarget key:

KUBECONFIG=.kcp/admin.kubeconfig kubectl get synctarget cluster1 --output=jsonpath={.metadata.labels."internal\.workload\.kcp\.io\/key"}

Now we need the downstream POD name:

kubectl get pods --all-namespaces -l app=kuard-tunneler --output=jsonpath={.items..metadata.name}

Now that we have the Upsyncer VW URL, the Synctarget Key and the POD name we can upsync a "fake" POD (this will likely be done in the future by the upsyncer controller), please replace the :

cat <<EOF | KUBECONFIG=.kcp/admin.kubeconfig kubectl --server=<upsyncerVW url >/clusters/root:myorg:myworkspace apply -n default --validate=false -f -
apiVersion: v1
kind: Pod
metadata:
  name: <downstream-pod-name>
  namespace: default
  labels:
    app: kuard-tunneler
    state.workload.kcp.io/<synctargetkey>: Upsync
spec:
  containers:
    - name: kuard-amd64
EOF

If everything went Ok, you should be able to retrieve the POD in KCP:

$ KUBECONFIG=.kcp/admin.kubeconfig kubectl get pods
NAME                             READY   STATUS   RESTARTS   AGE
kuard-tunneler-f4bd64bd9-5p7jk   0/0              0          5s

Now you should be able to get the pods logs or exec a command:

$ KUBECONFIG=.kcp/admin.kubeconfig kubectl logs kuard-tunneler-f4bd64bd9-5p7jk
2022/11/25 17:33:41 Starting kuard version: v0.10.0-blue
2022/11/25 17:33:41 **********************************************************************
2022/11/25 17:33:41 * WARNING: This server may expose sensitive
2022/11/25 17:33:41 * and secret information. Be careful.
2022/11/25 17:33:41 **********************************************************************
2022/11/25 17:33:41 Config: 
{
  "address": ":8080",
  "debug": false,
  "debug-sitedata-dir": "./sitedata",
  "keygen": {
    "enable": false,
    "exit-code": 0,
    "exit-on-complete": false,
    "memq-queue": "",
    "memq-server": "",
    "num-to-gen": 0,
    "time-to-run": 0

...

Related issue(s)

Fixes ##1976

@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 23, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 23, 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 force-pushed the jmprusi/syncer-tunnels-pod-logs branch 10 times, most recently from c1e8141 to cf5750c Compare November 25, 2022 17:44
@jmprusi jmprusi marked this pull request as ready for review November 25, 2022 17:59
@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 Nov 25, 2022
@openshift-ci openshift-ci bot requested review from csams and shawn-hurley November 25, 2022 17:59
@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch 2 times, most recently from bd42b0a to 421718a Compare November 27, 2022 23:32
Copy link
Member

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

Should we have unit tests ?

@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch 2 times, most recently from b431132 to 43fe979 Compare December 1, 2022 10:08
Copy link
Member

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

Maybe move your handler in a dedicated file ? (like for the Home workspaces handler)

@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch from 43fe979 to 1fa746d Compare December 1, 2022 14:54
@ncdc
Copy link
Member

ncdc commented Dec 1, 2022

/retitle ✨ Support for pod logs and other subresources

@openshift-ci openshift-ci bot changed the title ✨ Support for POD logs and other sub resources. ✨ Support for pod logs and other subresources Dec 1, 2022
@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch 2 times, most recently from c4bc28d to 3c1265c Compare December 1, 2022 17:38
@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch 4 times, most recently from 14073bb to 7199a2e Compare January 24, 2023 17:13
@jmprusi
Copy link
Member Author

jmprusi commented Jan 24, 2023

/retest

@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch 2 times, most recently from 6ec7aaa to 708c5e8 Compare January 26, 2023 13:45
@jmprusi jmprusi force-pushed the jmprusi/syncer-tunnels-pod-logs branch from 708c5e8 to 4c4e687 Compare January 26, 2023 15:17
@davidfestal
Copy link
Member

/lgtm

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

@sttts Would you give a last look and approve ?

@jmprusi jmprusi requested a review from sttts January 26, 2023 20:24
@sttts
Copy link
Member

sttts commented Jan 30, 2023

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 30, 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 Jan 30, 2023
@jmprusi
Copy link
Member Author

jmprusi commented Jan 30, 2023

known flake kcp-dev/contrib-tmc#77, retrying

/retest

@davidfestal
Copy link
Member

/retest

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. area/transparent-multi-cluster Related to scheduling of workloads into pclusters. 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.

6 participants