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
tests: Support for kbs setup on kcli #9273
Conversation
@@ -224,8 +224,6 @@ function kbs_k8s_deploy() { | |||
popd | |||
echo "::endgroup::" | |||
|
|||
[ -n "$ingress" ] && _handle_ingress "$ingress" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ldoktor !
AKS handler requires to be evoked before calling deploy-kbs.sh
. It will create the ingress.yaml (https://github.com/kata-containers/kata-containers/blob/main/tests/integration/kubernetes/confidential_kbs.sh#L401) under "${COCO_KBS_DIR}/config/kubernetes/overlays"
and adjust the kustomize file (https://github.com/kata-containers/kata-containers/blob/main/tests/integration/kubernetes/confidential_kbs.sh#L408). Thus the ingress deployment is applied alongside the KBS by deploy-kbs.sh
.
Can we follow that approach with NodePort instead of calling kubectl expose...
?
One extra advance is that you won't need to do the deleting of nodePort ingress on k8s_kbs_delete()
- https://github.com/kata-containers/kata-containers/blob/main/tests/integration/kubernetes/confidential_kbs.sh#L162
Changes:
|
elif kubectl get svc kbs-nodeport -n "$KBS_NS" &>/dev/null; then | ||
local host | ||
host=$(kubectl get nodes -o jsonpath='{.items[0].status.addresses[?(@.type=="ExternalIP")].address}' -n "$KBS_NS") | ||
[ -z "$host"] && host=$(oc get nodes -o jsonpath='{.items[0].status.addresses[?(@.type=="InternalIP")].address}' -n "$KBS_NS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ldoktor !
I looked at the code and I didn't find anything wrong. Then I give a try on my local machine but it failed to return the service address. Luckily I changed of laptop recently and I don't have oc
installed yet, so that's the problem it is using oc
here :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oups, let me fix that (oc is shorter than kubectl...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new version worked for me @ldoktor !
this can be used on kcli or other systems where cluster nodes are accessible from all places where the tests are running. Fixes: kata-containers#9272 Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
the service might not listen on the default port, use the full service address to ensure we are talking to the right resource. Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Changes:
|
|
||
cat > nodeport_service.yaml <<EOF | ||
# Service to expose the KBS on nodes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ldoktor the only thing that I would change on this PR is to contribute the nodeport_service.yaml to https://github.com/confidential-containers/trustee/tree/main/kbs/config/kubernetes . So it lives alongside the ingress deployment yaml for aks. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it but unlike AKS the nodeport service is quite hackish and mainly supported for debugging/testing purposes. I don't think it'd be wise to promote it as another "option".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good and fair point @ldoktor !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ldoktor
Hi @fitzthum ! Have you had a chance to test this on kubeadm and k3s? |
Not yet unfortunately and I am sick today so I am not sure when I will be able to get to it. |
@ldoktor @wainersm I am trying to test on a fresh baremetal, however, when I am doing this
To avoid that error I found out that I need to do the
But I am getting this errors
I try to follow the github workflow with the steps, I could not find something that was using deploy-kata-kcli, not sure if something else is missing |
Hi @GabyCT , The instructions on the PR's description is for a cluster built with
If you are not running on TDX then it's fine, I suppose the |
@wainersm thanks for the info I just run it on a non-TDX environment but following your instructions and seems ok
|
Great! Thanks for you time running it, @GabyCT ! |
/test |
this adds a support for
kcli
totests/integration/kubernetes/gha-run.sh deploy-coco-kbs
. It uses nodeport feature to expose the service from localhost as well as between the nodes, one can test it by: