Create feature test for scale-in and node reuse by jan-est · Pull Request #633 · metal3-io/metal3-dev-env

jan-est · 2021-04-14T08:36:58Z

This PR creates feature test for testing scale-in and node reuse features. Both features are tested with KCP and MachineDeployment.

jan-est · 2021-04-15T05:39:24Z

/test-centos-integration
/test-integration

furkatgofurov7 · 2021-04-18T08:59:15Z

/test-features

furkatgofurov7 · 2021-04-23T11:32:37Z

I have added test cases for both KCP and MD test scenarios. I had difficulty putting all steps to test both CAPI objects so I separated test scenarios for KCP and MD. Let me know if I can optimize it or you have a better solution to this.

furkatgofurov7 · 2021-04-23T15:29:59Z

/hold
#639 should go in first.

furkatgofurov7 · 2021-04-29T13:01:52Z

/unhold

furkatgofurov7 · 2021-05-20T13:55:03Z

/test-features
/test-upgrade-features

furkatgofurov7 · 2021-05-20T14:21:53Z

/test-features
/test-upgrade-features

fmuyassarov · 2021-05-20T16:10:58Z

+
+  - name: Scale worker down to 0
+    community.kubernetes.k8s_scale:
+      api_version: MD_API_VERSION


This needs to be replaced with the actual apiVersion of MachineDeployment

Sorry, missed that it was a variable. {{}} are missing?

furkatgofurov7 · 2021-05-20T16:30:25Z

/test-features

furkatgofurov7 · 2021-05-20T19:10:35Z

/test-features
/test-upgrade-features
/test-centos-integration
/test-integration

furkatgofurov7 · 2021-05-20T19:10:45Z

/unhold

furkatgofurov7 · 2021-05-20T20:52:22Z

/test-features

furkatgofurov7 · 2021-05-21T06:40:47Z

Tests failing not even reaching the actual parts of code to be tested in this patch, re-triggering.

/test-features

furkatgofurov7 · 2021-05-23T17:58:40Z

/test-features

fmuyassarov

This looks good to me. One last comment in line
/approve

fmuyassarov · 2021-05-24T06:36:19Z

+    register: diff_mapping
+    failed_when: diff_mapping.rc == 1
+
+  - name: Clean up any KubeadmControlPlane node reuse test scenario related temp files.


This could also work

- name: Remove file (delete file) ansible.builtin.file: path: "{{ item }}" state: absent with_items: - /tmp/before_upgrade_mapping.txt - /tmp/after_upgrade_mapping.txt

fmuyassarov · 2021-05-24T06:36:53Z

+    failed_when: diff_mapping.rc == 1
+
+  - name: Clean up any MachineDeployment node reuse test scenario related temp files.
+    shell: rm /tmp/before_upgrade_mapping_md.txt /tmp/after_upgrade_mapping_md.txt


metal3-io-bot · 2021-05-24T06:48:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmuyassarov, jan-est

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [fmuyassarov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kashifest

LGTM. I had a hard time understanding which of the tests are for scale in and which for node-reuse, can we add more specific names for the tests ?

kashifest · 2021-05-24T06:49:47Z

 # ${CLUSTER_API_REPO}/cmd/clusterctl/hack/create-local-repository.py
 # For CAPI release v0.3.12, the folder v0.3.8 is created

-export UPGRADED_K8S_VERSION="v1.21.1"


why are we removing this?

In scale-in && node reuse tests we need the same kind of var in order to upgrade k8s version in the test. Currently UPGRADED_K8S_VERSION is only used in upgrade tests, and lives inside the upgrade specific files, so it was removed/moved to upper root file(common.sh) so any test can access it.

kashifest · 2021-05-24T06:54:03Z

 export CLUSTER_NAME=${CLUSTER_NAME:-"test1"}
 export CLUSTER_APIENDPOINT_IP=${CLUSTER_APIENDPOINT_IP:-"192.168.111.249"}
 export KUBERNETES_VERSION=${KUBERNETES_VERSION:-"v1.21.0"}
+export UPGRADED_K8S_VERSION=${UPGRADED_K8S_VERSION:-"v1.21.1"}


why do we need this here?

Please check the above comment. We need it define it globally here, so all dependent tests(upgrade, scale-in && node reuse) on this var can use it.

furkatgofurov7 · 2021-05-24T07:07:15Z

LGTM. I had a hard time understanding which of the tests are for scale in and which for node-reuse, can we add more specific names for the tests ?

The thing is, we are testing scale-in feature within the the node reuse feature. During the tests, we first set max_surge=0 and upgrade KCPs' 1 by 1, and at this point we check both features:

node reuse: is same BMH reused?
scale-in: is same machine deleted and re-created rather than new machine being created?

So, as you see, it is mixed with each other, and hard to separate the tests from each other. So, that's why we called it just 'node reuse' in the tasks, but in read.me we do note that scale-in is also tested during the node_reuse_test. Would that make sense to you @kashifest ?

kashifest · 2021-05-24T07:30:40Z

node reuse: is same BMH reused?

scale-in: is same machine deleted and re-created rather than new machine being created?

Are you doing these checks in the same ansible task? Seems like one is checking bmh and one is machine, atleast those tasks can have appropriate names?

kashifest · 2021-05-24T07:44:52Z

node reuse: is same BMH reused?

scale-in: is same machine deleted and re-created rather than new machine being created?

Are you doing these checks in the same ansible task? Seems like one is checking bmh and one is machine, atleast those tasks can have appropriate names?

We can take care of this change in a future PR since this is a cosmetic change and the PR has passed all the tests.

/lgtm

furkatgofurov7 · 2021-05-24T07:48:34Z

Are you doing these checks in the same ansible task? Seems like one is checking bmh and one is machine, atleast those tasks can have appropriate names?

@kashifest yes, in the same task. Good point, sure, I will update the names accordingly in the task based on the which feature we are testing (scale-in or node reuse) since they are all mixed in a single task it can be hard to debug in the future.

This could also work

@fmuyassarov I will tackle both comments in the follow up PR, thanks all for review.

metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 14, 2021

furkatgofurov7 force-pushed the nodereuse-integration-test branch 4 times, most recently from e7f769b to 7ce7355 Compare April 18, 2021 21:02

fmuyassarov reviewed Apr 20, 2021

View reviewed changes

furkatgofurov7 force-pushed the nodereuse-integration-test branch 3 times, most recently from 89bd3f9 to 4c43fc6 Compare April 23, 2021 11:28

jan-est commented Apr 23, 2021

View reviewed changes

Comment thread vm-setup/roles/v1aX_integration_test/tasks/node_reuse_kcp.yml Outdated

furkatgofurov7 force-pushed the nodereuse-integration-test branch 3 times, most recently from bcbda0f to fc11d31 Compare April 23, 2021 15:25

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 23, 2021

furkatgofurov7 force-pushed the nodereuse-integration-test branch 3 times, most recently from d054ab9 to be4b342 Compare April 29, 2021 13:00

furkatgofurov7 force-pushed the nodereuse-integration-test branch from be4b342 to 51586b9 Compare April 29, 2021 14:28

metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 2, 2021

jan-est force-pushed the nodereuse-integration-test branch from 51586b9 to 8c8c5a7 Compare May 3, 2021 06:17

metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 3, 2021

furkatgofurov7 force-pushed the nodereuse-integration-test branch 2 times, most recently from 84366cd to 7c7f93a Compare May 3, 2021 10:32

metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 3, 2021

furkatgofurov7 force-pushed the nodereuse-integration-test branch from 7c7f93a to c51d8a1 Compare May 3, 2021 10:47

furkatgofurov7 force-pushed the nodereuse-integration-test branch from 2d86073 to b623f90 Compare May 20, 2021 14:21

fmuyassarov suggested changes May 20, 2021

View reviewed changes

furkatgofurov7 force-pushed the nodereuse-integration-test branch from b623f90 to 1e344f4 Compare May 20, 2021 16:29

Address review comments

7c2e460

furkatgofurov7 force-pushed the nodereuse-integration-test branch from 1e344f4 to 7c2e460 Compare May 20, 2021 19:09

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 20, 2021

fmuyassarov reviewed May 24, 2021

View reviewed changes

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2021

kashifest reviewed May 24, 2021

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label May 24, 2021

metal3-io-bot merged commit 7887985 into metal3-io:master May 24, 2021

furkatgofurov7 added a commit to Nordix/metal3-dev-env that referenced this pull request May 24, 2021

Follow up to metal3-io#633

ecc711f

furkatgofurov7 mentioned this pull request May 24, 2021

Improve scale-in and node reuse feature tests #663

Merged

furkatgofurov7 added a commit to Nordix/metal3-dev-env that referenced this pull request May 24, 2021

Follow up to metal3-io#633

6018fef

furkatgofurov7 added a commit to Nordix/metal3-dev-env that referenced this pull request May 25, 2021

Follow up to metal3-io#633

890dc63

furkatgofurov7 added a commit to Nordix/metal3-dev-env that referenced this pull request May 25, 2021

Follow up to metal3-io#633

5d2d1f9

jan-est deleted the nodereuse-integration-test branch October 4, 2021 05:57

Conversation

jan-est commented Apr 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jan-est commented Apr 15, 2021

Uh oh!

furkatgofurov7 commented Apr 18, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

furkatgofurov7 commented Apr 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

furkatgofurov7 commented Apr 23, 2021

Uh oh!

furkatgofurov7 commented Apr 29, 2021

Uh oh!

furkatgofurov7 commented May 20, 2021

Uh oh!

furkatgofurov7 commented May 20, 2021

Uh oh!

fmuyassarov May 20, 2021

Choose a reason for hiding this comment

Uh oh!

fmuyassarov May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furkatgofurov7 commented May 20, 2021

Uh oh!

furkatgofurov7 commented May 20, 2021

Uh oh!

furkatgofurov7 commented May 20, 2021

Uh oh!

furkatgofurov7 commented May 20, 2021

Uh oh!

furkatgofurov7 commented May 21, 2021

Uh oh!

furkatgofurov7 commented May 23, 2021

Uh oh!

fmuyassarov left a comment

Choose a reason for hiding this comment

Uh oh!

fmuyassarov May 24, 2021

Choose a reason for hiding this comment

Uh oh!

fmuyassarov May 24, 2021

Choose a reason for hiding this comment

Uh oh!

metal3-io-bot commented May 24, 2021

Uh oh!

kashifest left a comment

Choose a reason for hiding this comment

Uh oh!

kashifest May 24, 2021

Choose a reason for hiding this comment

Uh oh!

furkatgofurov7 May 24, 2021

Choose a reason for hiding this comment

Uh oh!

kashifest May 24, 2021

Choose a reason for hiding this comment

Uh oh!

furkatgofurov7 May 24, 2021

Choose a reason for hiding this comment

Uh oh!

furkatgofurov7 commented May 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kashifest commented May 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

jan-est commented Apr 14, 2021 •

edited

Loading

furkatgofurov7 commented Apr 23, 2021 •

edited

Loading

fmuyassarov May 20, 2021 •

edited

Loading

furkatgofurov7 commented May 24, 2021 •

edited

Loading

kashifest commented May 24, 2021 •

edited

Loading