ci: add k8s uninstallation to metrics tests #4206

dborquez · 2021-11-22T15:44:23Z

IPerf bench runs on top of a K8s setup, for instance ci metrics
needs to clean the k8s setup, before start the test.

Signed-off-by: David Esparza david.esparza.borquez@intel.com

GabyCT · 2021-11-22T15:49:46Z

@dborquez I think this should be part of the commit where you are enabling iperf metrics because I have a lot of questions, 1. How frequent is the test failing in removing or installing k8s on the metrics CI? 2. How stable the results are? 3. Do we need k8s or ctr?

.ci/lib.sh

jodh-intel · 2021-12-23T10:14:26Z

@dborquez - Please can you address the outstanding comments on this PR when you're back.

jodh-intel · 2022-01-06T14:45:36Z

Ping @dborquez.

jodh-intel · 2022-01-11T08:31:42Z

@dborquez - please can you update?

dborquez · 2022-01-18T21:25:02Z

@jodh-intel yes WIP

jodh-intel

Thanks @dborquez.

lgtm

dborquez · 2022-01-26T11:23:14Z

@dborquez I think this should be part of the commit where you are enabling iperf metrics because I have a lot of questions, 1. How frequent is the test failing in removing or installing k8s on the metrics CI? 2. How stable the results are? 3. Do we need k8s or ctr?

@GabyCT thanks for the remarks.
The idea is to enable the iperf K8s version that already exist in metrics. See the issue #3475.
So this PR is to make the Metrics CI-job can remove K8s before to start the iperf test.
In a following PR I will enable iperf and then determine if the test can run continuously with no issues

fidencio · 2022-01-26T12:45:09Z

A head's up that may affect your PR.

In the Architecture Committee meeting from January 25th, 2022, the Architecture Committee has agreed on using the "Dismiss stale pull request approvals when new commits are pushed" configuration from GitHub. It basically means that if your PR has been rebased or updated, the approvals given will be erased.

In order to minimize traumas due to the new approach, please, consider adding a note on the changes done before the rebase / force-push, and also pinging the reviewers for a subsequent round of reviews.

Thanks for your understanding!

Related issue: kata-containers/community#249

GabyCT · 2022-01-26T15:30:16Z

@dborquez I think this should be part of the commit where you are enabling iperf metrics because I have a lot of questions, 1. How frequent is the test failing in removing or installing k8s on the metrics CI? 2. How stable the results are? 3. Do we need k8s or ctr?

@GabyCT thanks for the remarks. The idea is to enable the iperf K8s version that already exist in metrics. See the issue #3475. So this PR is to make the Metrics CI-job can remove K8s before to start the iperf test. In a following PR I will enable iperf and then determine if the test can run continuously with no issues

@dborquez currently the METRICS CI is installing and cleaning k8s without these additional steps that you are implementing here at this PR, we do not know if the cleaning and installing is being done without an issues because we do not have any test that we can prove that works fine, you can do another commit here in this PR where you enable the iperf test and then you will see if the clean that you are implementing here in this PR will actually work or otherwise we can merge this PR and later you will find out that is not working properly or it is working...but it is your call

dborquez · 2022-01-26T15:56:46Z

@fidencio thank you, I'll follow the new approach.

dborquez · 2022-01-26T16:00:28Z

@dborquez I think this should be part of the commit where you are enabling iperf metrics because I have a lot of questions, 1. How frequent is the test failing in removing or installing k8s on the metrics CI? 2. How stable the results are? 3. Do we need k8s or ctr?

@GabyCT thanks for the remarks. The idea is to enable the iperf K8s version that already exist in metrics. See the issue #3475. So this PR is to make the Metrics CI-job can remove K8s before to start the iperf test. In a following PR I will enable iperf and then determine if the test can run continuously with no issues

@dborquez currently the METRICS CI is installing and cleaning k8s without these additional steps that you are implementing here at this PR, we do not know if the cleaning and installing is being done without an issues because we do not have any test that we can prove that works fine, you can do another commit here in this PR where you enable the iperf test and then you will see if the clean that you are implementing here in this PR will actually work or otherwise we can merge this PR and later you will find out that is not working properly or it is working...but it is your call

@GabyCT thanks, I will add a new commit to this PR with the iperf enablement.

dborquez · 2022-02-09T17:54:07Z

/test

GabyCT · 2022-02-09T18:02:52Z

@dborquez if you want to test and run only metrics try with /test-ubuntu-metrics also if you comment the other tests that will fail because it will look for the results here https://github.com/kata-containers/tests/blob/main/cmd/checkmetrics/ci_worker/checkmetrics-json-qemu-kata-metric5.toml you need to also add there something for iperf like 0b3e935

dborquez · 2022-02-09T19:21:28Z

@dborquez if you want to test and run only metrics try with /test-ubuntu-metrics also if you comment the other tests that will fail because it will look for the results here https://github.com/kata-containers/tests/blob/main/cmd/checkmetrics/ci_worker/checkmetrics-json-qemu-kata-metric5.toml you need to also add there something for iperf like 0b3e935

@GabyCT thanks for the inputs. Improving the patch with recommended ci settings.

dborquez · 2022-02-17T04:37:28Z

/test-ubuntu-metrics

dborquez · 2022-02-17T06:26:40Z

/test-ubuntu-metrics

dborquez · 2022-02-17T06:44:05Z

/test-ubuntu-metrics

jodh-intel · 2022-02-17T08:48:18Z

Any update on this @dborquez ?

dborquez · 2022-02-17T14:18:09Z

Any update on this @dborquez ?

WIP, debugging k8s that hangs up for ch metrics:
error execution phase preflight: [preflight] Some fatal errors occurred: 01:34:45 [ERROR Port-10250]: Port 10250 is in use

dborquez · 2022-02-17T14:46:15Z

/test-ubuntu-metrics

dborquez · 2022-02-17T21:46:16Z

/test-ubuntu-metrics

dborquez · 2022-02-17T22:25:33Z

/test-ubuntu-metrics

dborquez · 2022-02-17T22:43:56Z

/test-ubuntu-metrics

dborquez · 2022-02-17T23:25:44Z

/test-ubuntu-metrics

dborquez · 2022-02-18T04:00:46Z

/test-ubuntu-metrics

dborquez · 2022-02-18T05:17:36Z

/test-ubuntu-metrics

dborquez · 2022-02-18T05:49:50Z

@jodh-intel hello, could you please review this PR?

dborquez · 2022-02-18T05:52:21Z

/test

jodh-intel

Thanks @dborquez.

.ci/lib.sh

Iperf runs on top of a K8s setup, for instance ci metrics needs to clean the k8s setup, before start the test. Fixes: kata-containers#4205 Signed-off-by: David Esparza <david.esparza.borquez@intel.com>

dborquez · 2022-02-18T13:52:31Z

/test-ubuntu-metrics

jodh-intel

Thanks @dborquez.

lgtm

GabyCT · 2022-02-18T16:13:30Z

/test

GabyCT · 2022-02-18T16:11:21Z

.ci/run.sh

@@ -102,7 +102,6 @@ case "${CI_JOB}" in
 		sudo -E ln -sf "${config_path}/configuration-qemu.toml" "${config_path}/configuration.toml"
 		echo "INFO: Running qemu metrics tests"
 		sudo -E PATH="$PATH" ".ci/run_metrics_PR_ci.sh"
-		echo "INFO: Running cloud hypervisor metrics tests"


why do you remove this?

I think is useful to have it to mark the cloud hypervisor start

@GabyCT , that info msg is located before the test starts, in the same way the msg for qemu does:

tests/.ci/run.sh

Line 114 in f46c370

echo "INFO: Running cloud hypervisor metrics tests"

dborquez · 2022-02-23T22:06:58Z

/test-ubuntu-metrics

dborquez added needs-review Needs to be assessed by the team. do-not-merge PR has problems or depends on another labels Nov 22, 2021

dborquez requested a review from GabyCT November 22, 2021 15:44

dborquez added wip Work in Progress (PR incomplete - needs more work or rework) and removed needs-review Needs to be assessed by the team. labels Nov 25, 2021

jodh-intel reviewed Nov 29, 2021

View reviewed changes

.ci/lib.sh Show resolved Hide resolved

.ci/lib.sh Outdated Show resolved Hide resolved

dborquez force-pushed the clean_k8s_ci_metrics branch from 8933e42 to e2bf1d3 Compare January 26, 2022 10:53

jodh-intel previously approved these changes Jan 26, 2022

View reviewed changes

dborquez removed the wip Work in Progress (PR incomplete - needs more work or rework) label Jan 26, 2022

GabyCT previously approved these changes Jan 26, 2022

View reviewed changes

dborquez dismissed stale reviews from GabyCT and jodh-intel via aff5f99 February 9, 2022 16:35

dborquez force-pushed the clean_k8s_ci_metrics branch 2 times, most recently from aff5f99 to 5bf6165 Compare February 9, 2022 16:53

dborquez added do-not-merge PR has problems or depends on another and removed do-not-merge PR has problems or depends on another labels Feb 9, 2022

dborquez force-pushed the clean_k8s_ci_metrics branch from 5bf6165 to 32cbc1f Compare February 9, 2022 22:58

dborquez force-pushed the clean_k8s_ci_metrics branch from b1104f9 to f466195 Compare February 17, 2022 04:35

dborquez force-pushed the clean_k8s_ci_metrics branch from f466195 to 008173b Compare February 17, 2022 06:26

dborquez force-pushed the clean_k8s_ci_metrics branch from 008173b to 24fb6d5 Compare February 17, 2022 14:45

dborquez force-pushed the clean_k8s_ci_metrics branch from 24fb6d5 to a94dcd2 Compare February 17, 2022 21:34

dborquez force-pushed the clean_k8s_ci_metrics branch from a94dcd2 to 8d1a3fb Compare February 17, 2022 22:24

dborquez removed the do-not-merge PR has problems or depends on another label Feb 18, 2022

jodh-intel previously approved these changes Feb 18, 2022

View reviewed changes

.ci/lib.sh Outdated Show resolved Hide resolved

.ci/lib.sh Outdated Show resolved Hide resolved

ci: Enable Iperf bench

f46c370

Iperf runs on top of a K8s setup, for instance ci metrics needs to clean the k8s setup, before start the test. Fixes: kata-containers#4205 Signed-off-by: David Esparza <david.esparza.borquez@intel.com>

dborquez dismissed jodh-intel’s stale review via f46c370 February 18, 2022 13:51

dborquez force-pushed the clean_k8s_ci_metrics branch from 8d1a3fb to f46c370 Compare February 18, 2022 13:51

jodh-intel approved these changes Feb 18, 2022

View reviewed changes

GabyCT approved these changes Feb 18, 2022

View reviewed changes

GabyCT merged commit d2b396a into kata-containers:main Feb 18, 2022

ci: add k8s uninstallation to metrics tests #4206

ci: add k8s uninstallation to metrics tests #4206

Conversation

dborquez commented Nov 22, 2021

GabyCT commented Nov 22, 2021

jodh-intel commented Dec 23, 2021

jodh-intel commented Jan 6, 2022

jodh-intel commented Jan 11, 2022

dborquez commented Jan 18, 2022

jodh-intel left a comment

Choose a reason for hiding this comment

dborquez commented Jan 26, 2022

fidencio commented Jan 26, 2022

GabyCT commented Jan 26, 2022 • edited

dborquez commented Jan 26, 2022

dborquez commented Jan 26, 2022

dborquez commented Feb 9, 2022

GabyCT commented Feb 9, 2022

dborquez commented Feb 9, 2022 • edited

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

jodh-intel commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 17, 2022

dborquez commented Feb 18, 2022

dborquez commented Feb 18, 2022

dborquez commented Feb 18, 2022

dborquez commented Feb 18, 2022

jodh-intel left a comment

Choose a reason for hiding this comment

dborquez commented Feb 18, 2022

jodh-intel left a comment

Choose a reason for hiding this comment

GabyCT commented Feb 18, 2022

GabyCT Feb 18, 2022

Choose a reason for hiding this comment

GabyCT Feb 18, 2022

Choose a reason for hiding this comment

dborquez Feb 18, 2022 • edited

Choose a reason for hiding this comment

dborquez commented Feb 23, 2022

GabyCT commented Jan 26, 2022 •

edited

dborquez commented Feb 9, 2022 •

edited

dborquez Feb 18, 2022 •

edited