Skip to content

USHIFT-1199: add automated tests for etcd memory limit management#1735

Merged
openshift-merge-robot merged 1 commit intoopenshift:mainfrom
dhellmann:USHIFT-1199-robot-test-etcd-memory-limit
Jun 1, 2023
Merged

USHIFT-1199: add automated tests for etcd memory limit management#1735
openshift-merge-robot merged 1 commit intoopenshift:mainfrom
dhellmann:USHIFT-1199-robot-test-etcd-memory-limit

Conversation

@dhellmann
Copy link
Copy Markdown
Contributor

No description provided.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 29, 2023
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Apr 29, 2023

@dhellmann: This pull request references USHIFT-1199 which is a valid jira issue.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci Bot requested review from copejon and pmtk April 29, 2023 19:46
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 29, 2023
@dhellmann
Copy link
Copy Markdown
Contributor Author

/hold

These tests depend on #1734

These tests introduce instability to the overall test run by restarting microshift and not waiting long enough for it to restart.

@dhellmann dhellmann changed the title USHIFT-1199: add automated tests for etcd memory limit management WIP: USHIFT-1199: add automated tests for etcd memory limit management Apr 29, 2023
@openshift-ci openshift-ci Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Apr 29, 2023
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch from 9c5902e to 9a45753 Compare April 29, 2023 22:10
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2023
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch from 9a45753 to 44da87a Compare May 6, 2023 19:53
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2023
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch 3 times, most recently from 281b088 to d789ebc Compare May 10, 2023 13:40
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch from d789ebc to fece1d7 Compare May 16, 2023 19:23
@pacevedom
Copy link
Copy Markdown
Contributor

/assign @pacevedom

@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch 5 times, most recently from 0b445dd to 151dba5 Compare May 25, 2023 16:14
@dhellmann dhellmann removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 25, 2023
@dhellmann dhellmann changed the title WIP: USHIFT-1199: add automated tests for etcd memory limit management USHIFT-1199: add automated tests for etcd memory limit management May 25, 2023
@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 May 25, 2023
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch from 151dba5 to 4aeb19d Compare May 25, 2023 16:39
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch 3 times, most recently from 9547aab to 15ec680 Compare May 26, 2023 19:53
@dhellmann
Copy link
Copy Markdown
Contributor Author

/retest-required

@dhellmann
Copy link
Copy Markdown
Contributor Author

/hold

The e2e tests are not running on this PR because only test code is being changed. See openshift/release#39797

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 28, 2023
@dhellmann
Copy link
Copy Markdown
Contributor Author

/retest-required

@dhellmann
Copy link
Copy Markdown
Contributor Author

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 29, 2023
@dhellmann
Copy link
Copy Markdown
Contributor Author

/test microshift-e2e-arm

@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch 3 times, most recently from 1fb0962 to 32f6a74 Compare May 30, 2023 20:56
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 30, 2023

@dhellmann: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openshift-conformance-reduced-arm 4aeb19db6ba1e1b8c5ba1528f471e1e0154e713e link false /test e2e-openshift-conformance-reduced-arm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch from 32f6a74 to fd4901a Compare May 30, 2023 22:58
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about cat /etc/microshift/config.yaml 2>/dev/null || true? This way we can check for rc==0 and avoid mistaking ssh errors for an empty configuration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we always return true, we would have to look at the output to see if we got any content. Is that what you mean? I'm not sure when we would have an empty file.

Comment thread test/resources/microshift-config.resource Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MicroShift runs as root so it will be able to read anything, isnt it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does, but we want to make sure that file is owned by root for security and because that's what we would expect users to do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually it does work even if not in ostree. You will see an error about not being in an ostree host but thats only cosmetic. Checks run and return codes reflect situation. Needs some refactoring though, as it always expects topolvm to be up, even if there is no lvmd.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I tried the script and it exited because I was not on an ostree system. Maybe I was running the wrong one.

Comment thread test/resources/microshift-process.resource Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Greenboot tests do this in a different way, will ping you when its up for review. Actually it would benefit from some of the work in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@pmtk has some "wait" logic in one of his PRs, too. It would be ideal if we could come up with 1 good strong implementation of that.

Comment thread test/resources/systemd.resource Outdated
Comment thread test/resources/systemd.resource Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is already covered by sos report, isnt it? It doesnt even matter if the test succeeds or not (it fails here, though). You are going to get a huge output from this, mixed with the other tests/keywords output.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We may want to cut this down, but I found it useful in debugging to see the logs from the process at the time of the command inline. I could make it 20 lines?

Comment thread test/suites/etcd.robot Outdated
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
@dhellmann dhellmann force-pushed the USHIFT-1199-robot-test-etcd-memory-limit branch from fd4901a to 8a0c986 Compare May 31, 2023 22:48
Copy link
Copy Markdown
Contributor

@pacevedom pacevedom left a comment

Choose a reason for hiding this comment

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

Will address comments in further PRs, as the main target of the PR looks good. Merging for progress and lets keep iterating the common functions.
/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 1, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jun 1, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, pacevedom

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 [dhellmann,pacevedom]

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

@openshift-merge-robot openshift-merge-robot merged commit 08eae1e into openshift:main Jun 1, 2023
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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants