peerpod ctrl #283

snir911 · 2023-04-04T13:26:28Z

This is integration of peerpod-ctrl in this controller

* This is rebased on top of #282 * currently supports only aws (waiting for azure upstream merge)
To install:

build and install controller normally, and KataConfig with enablePeerPods: true

make sure caa pods are up and running

run kubectl apply -f hack/caa-rbac.yaml workaround to set rbac for the caa pods (I'm not sure if they require to be restarted)

change the caa pod images from quay.io/confidential-containers/cloud-api-adaptor-aws to quay.io/confidential-containers/cloud-api-adaptor to make sure the peerpod-ctrl caa changes included (related also to make caa image url configurable #284)

then you should be able to run a (peer)pod and see PeerPod object created (it owned by the pod) and managed by the controller in case of deletion failure

TODOs:

make caa pod to be deployed with the correct rbac permissions

controller rbac for PeerPod modified manually, generate it using kubebuilder instead

remove hack patch

make sure it's rebased correctly once

This is all fixed, just build and test

azure is enabled but not tested

snir911 · 2023-04-23T12:47:51Z

Removing WIP, I added a commit to keep up with #286 however, I'm not sure it's correct as locally i'm failing to build after this change
cc @bpradipt @jensfr @cpmeadors

snir911 · 2023-04-24T12:44:59Z

removed the controller-tools patch as it will be fixed by:
#295

Makefile

bpradipt · 2023-04-24T16:57:45Z

config/manager/manager.yaml

@@ -30,6 +30,7 @@ spec:
      labels:
        control-plane: controller-manager
    spec:
+      hostNetwork: true


Is hostNetwork=true due to peerpod-ctrl and is this really needed ?

yes, AFAIR for it's needed in order to use Instance Metadata Service

main.go

cpmeadors · 2023-04-26T18:35:18Z

The PR description is really confusing. What does this PR actually implement/change? Is there a jira issue for this?

snir911 · 2023-04-27T10:10:18Z

This is the downstream integration part of the dangling resources controller in the sandboxed-container-operator, IIRC it's referred under the same Jira.
Also updated the commit msg to point to the upstream controller doc to make it clearer

Dockerfile

main.go

to avoid: vbom.ml/util@v0.0.0-20180919145318-efcd4e0f9787: unrecognized import path "vbom.ml/util": https fetch: Get "https://vbom.ml/util?go-get=1": dial tcp: lookup vbom.ml: no such host Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

controllers/openshift_controller.go

go.mod

main.go

gkurz · 2023-05-04T15:22:30Z

I don't feel like I'm qualified to do a thorough review of this PR, but I did not noticed anything that looked obviously wrong.

bpradipt · 2023-05-04T16:15:52Z

Just tested this on my Azure setup by following this sequence

Create a sample pod and watch the cloud-api-adaptor logs
Once VM is created, deleted the cloud-api-adaptor pod
Delete the sample pod
See all peerpod resources are deleted and verify from the admin console as well

bpradipt

/lgtm

in the current controller https://github.com/confidential-containers/cloud-api-adaptor/tree/staging/peerpod-ctrl Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

to support peerpod-ctrl: - peerpod-ctrl rbac (for manager itself) added using kubebuilder markers - peerpodconfig-ctrl rbac (for CAA DA) added using caa_rbac.yaml Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

openshift-ci · 2023-05-04T16:39:41Z

New changes are detected. LGTM label has been removed.

jensfr

lgtm, thank you @snir911

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 Apr 4, 2023

openshift-ci bot requested review from jensfr and littlejawa April 4, 2023 13:28

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 7, 2023

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 20, 2023

snir911 force-pushed the peerpod-ctrl branch from 82e691d to a8d40c0 Compare April 23, 2023 09:47

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2023

snir911 changed the title ~~WIP: peerpod ctrl~~ peerpod ctrl Apr 23, 2023

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 Apr 23, 2023

snir911 force-pushed the peerpod-ctrl branch from a8d40c0 to c89f3aa Compare April 24, 2023 12:43

bpradipt reviewed Apr 24, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

bpradipt reviewed Apr 24, 2023

View reviewed changes

main.go Outdated Show resolved Hide resolved

bpradipt reviewed Apr 24, 2023

View reviewed changes

main.go Outdated Show resolved Hide resolved

snir911 force-pushed the peerpod-ctrl branch from c89f3aa to e81a15b Compare April 25, 2023 05:59

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 26, 2023

snir911 force-pushed the peerpod-ctrl branch from e81a15b to d8cd80a Compare April 27, 2023 09:39

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2023

bpradipt reviewed Apr 27, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

bpradipt reviewed Apr 27, 2023

View reviewed changes

main.go Outdated Show resolved Hide resolved

bpradipt added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 27, 2023

go.mod: fix go mod error

f7b3d19

to avoid: vbom.ml/util@v0.0.0-20180919145318-efcd4e0f9787: unrecognized import path "vbom.ml/util": https fetch: Get "https://vbom.ml/util?go-get=1": dial tcp: lookup vbom.ml: no such host Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

snir911 force-pushed the peerpod-ctrl branch from d8cd80a to e88e0cf Compare May 2, 2023 11:39

bpradipt removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 2, 2023

bpradipt reviewed May 2, 2023

View reviewed changes

controllers/openshift_controller.go Show resolved Hide resolved

bpradipt reviewed May 2, 2023

View reviewed changes

go.mod Outdated Show resolved Hide resolved

bpradipt reviewed May 2, 2023

View reviewed changes

go.mod Outdated Show resolved Hide resolved

bpradipt requested review from gkurz and pmores May 2, 2023 12:25

snir911 force-pushed the peerpod-ctrl branch from e88e0cf to 2ac0308 Compare May 2, 2023 13:59

bpradipt reviewed May 2, 2023

View reviewed changes

main.go Outdated Show resolved Hide resolved

snir911 force-pushed the peerpod-ctrl branch 2 times, most recently from 05ff1bc to 77a44d2 Compare May 4, 2023 12:38

gkurz reviewed May 4, 2023

View reviewed changes

main.go Outdated Show resolved Hide resolved

bpradipt approved these changes May 4, 2023

View reviewed changes

openshift-ci bot assigned bpradipt May 4, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 4, 2023

snir911 added 3 commits May 4, 2023 19:39

controller: embed the peerpod-ctrl

40e524c

in the current controller https://github.com/confidential-containers/cloud-api-adaptor/tree/staging/peerpod-ctrl Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

controllers rbac manifests

ca51a94

to support peerpod-ctrl: - peerpod-ctrl rbac (for manager itself) added using kubebuilder markers - peerpodconfig-ctrl rbac (for CAA DA) added using caa_rbac.yaml Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

peerpodconfig-ctrl: update crd

d44a405

Signed-off-by: Snir Sheriber <ssheribe@redhat.com>

snir911 force-pushed the peerpod-ctrl branch from 77a44d2 to d44a405 Compare May 4, 2023 16:39

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 4, 2023

jensfr approved these changes May 5, 2023

View reviewed changes

jensfr merged commit e3a2b0e into openshift:peer-pods-tech-preview May 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

peerpod ctrl #283

peerpod ctrl #283

snir911 commented Apr 4, 2023 •

edited

snir911 commented Apr 23, 2023

snir911 commented Apr 24, 2023

bpradipt Apr 24, 2023

snir911 Apr 25, 2023

cpmeadors commented Apr 26, 2023

snir911 commented Apr 27, 2023

gkurz commented May 4, 2023

bpradipt commented May 4, 2023

bpradipt left a comment

openshift-ci bot commented May 4, 2023

jensfr left a comment

peerpod ctrl #283

peerpod ctrl #283

Conversation

snir911 commented Apr 4, 2023 • edited

snir911 commented Apr 23, 2023

snir911 commented Apr 24, 2023

bpradipt Apr 24, 2023

Choose a reason for hiding this comment

snir911 Apr 25, 2023

Choose a reason for hiding this comment

cpmeadors commented Apr 26, 2023

snir911 commented Apr 27, 2023

gkurz commented May 4, 2023

bpradipt commented May 4, 2023

bpradipt left a comment

Choose a reason for hiding this comment

openshift-ci bot commented May 4, 2023

jensfr left a comment

Choose a reason for hiding this comment

snir911 commented Apr 4, 2023 •

edited