-
Notifications
You must be signed in to change notification settings - Fork 228
USHIFT-1199: add automated tests for etcd memory limit management #1735
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| *** Settings *** | ||
| Documentation Keywords for running the microshift command line. | ||
|
|
||
| Library Process | ||
| Library String | ||
| Library OperatingSystem | ||
|
|
||
| Library SSHLibrary | ||
| Library ../resources/YAML.py | ||
|
|
||
|
|
||
| *** Keywords *** | ||
| Save Default MicroShift Config | ||
| [Documentation] Fetch the current config settings and preserve them as the default | ||
| ... | ||
| ... Sets the suite variable DEFAULT_MICROSHIFT_CONFIG to the text value | ||
| ... based on the contents of /etc/microshift/config.yaml, if it exists, or | ||
| ... an empty string if the file does not exist. | ||
| ... | ||
| ... This keyword is meant to be used from a Setup step. | ||
| ${stdout} ${rc}= Execute Command | ||
| ... cat /etc/microshift/config.yaml | ||
| ... sudo=True return_rc=True | ||
| IF ${rc} == 0 | ||
| Set Suite Variable \${DEFAULT_MICROSHIFT_CONFIG} ${stdout} | ||
| ELSE | ||
| Set Suite Variable \${DEFAULT_MICROSHIFT_CONFIG} ${EMPTY} | ||
| END | ||
|
|
||
| Restore Default MicroShift Config | ||
| [Documentation] Replace the microshift config file with the original defaults. | ||
| ... | ||
| ... If there was no configuration file originally, delete any file that is there now. | ||
| ${len}= Get Length ${DEFAULT_MICROSHIFT_CONFIG} | ||
| IF ${len} == 0 | ||
| # If there was no configuration file to start with, we do not want to create | ||
| # a new one, even if it is empty. | ||
| Clear MicroShift Config | ||
| ELSE | ||
| Upload MicroShift Config ${DEFAULT_MICROSHIFT_CONFIG} | ||
| END | ||
|
|
||
| Extend MicroShift Config | ||
| [Documentation] Return combination of default config and input argument as a string. | ||
| ... | ||
| ... The values are parsed as YAML and merged additively (no keys are deleted | ||
| ... and list values are extended but not replaced) by 'Yaml Merge'. | ||
| [Arguments] ${config} | ||
| ${merged}= Yaml Merge ${DEFAULT_MICROSHIFT_CONFIG} ${config} | ||
| RETURN ${merged} | ||
|
|
||
| Clear MicroShift Config | ||
| [Documentation] Remove any configuration file | ||
| ${stdout} ${rc}= Execute Command | ||
| ... rm -f /etc/microshift/config.yaml | ||
| ... sudo=True return_rc=True | ||
|
|
||
| Upload MicroShift Config # robocop: disable=too-many-calls-in-keyword | ||
| [Documentation] Upload a new configuration file to the MicroShift host | ||
| [Arguments] ${config_content} | ||
|
|
||
| ${rand}= Generate Random String | ||
| ${local_tmp}= Join Path /tmp ${rand} | ||
| Create File ${local_tmp} ${config_content} | ||
|
|
||
| ${rand}= Generate Random String | ||
| ${remote_tmp}= Join Path /tmp ${rand} | ||
| Put File ${local_tmp} ${remote_tmp} mode=0644 | ||
|
|
||
| Remove File ${local_tmp} | ||
|
|
||
| ${stdout} ${rc}= Execute Command | ||
| ... mv ${remote_tmp} /etc/microshift/config.yaml | ||
| ... sudo=True return_rc=True | ||
| Should Be Equal As Integers 0 ${rc} | ||
|
|
||
| ${stdout} ${rc}= Execute Command | ||
| ... chown root:root /etc/microshift/config.yaml | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| ... sudo=True return_rc=True | ||
| Should Be Equal As Integers 0 ${rc} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,8 +6,10 @@ Library String | |
| Library OperatingSystem | ||
|
|
||
| Library SSHLibrary | ||
| Resource ../resources/oc.resource | ||
| Resource ../resources/systemd.resource | ||
| Resource ../resources/microshift-host.resource | ||
| Library ../resources/YAML.py | ||
| Library ../resources/YAML.py | ||
|
|
||
|
|
||
| *** Keywords *** | ||
|
|
@@ -20,3 +22,42 @@ MicroShift Version | |
| Should Not Be Empty ${version_text} | ||
| ${version}= Yaml Parse ${version_text} | ||
| RETURN ${version} | ||
|
|
||
| MicroShift Is Ready | ||
| [Documentation] Check the /readyz endpoint | ||
| ${stdout}= Run With Kubeconfig oc get --raw='/readyz' | ||
| Should Be Equal As Strings ${stdout} ok strip_spaces=True | ||
|
|
||
| MicroShift Is Live | ||
| [Documentation] Check the /livez endpoint | ||
| ${stdout}= Run With Kubeconfig oc get --raw='/livez' | ||
| Should Be Equal As Strings ${stdout} ok strip_spaces=True | ||
|
|
||
| Wait For MicroShift | ||
| [Documentation] Wait for various checks to ensure MicroShift is online. | ||
| Wait Until Keyword Succeeds | ||
| ... 30x | ||
| ... 10s | ||
| ... MicroShift Is Ready | ||
| Wait Until Keyword Succeeds | ||
| ... 30x | ||
| ... 10s | ||
| ... MicroShift Is Live | ||
| # We could also wait for relevant pods. Can we restructure the | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| # greenboot check script to let us use it even when not on a | ||
| # greenboot host? | ||
|
|
||
| Restart MicroShift | ||
| [Documentation] Restart the MicroShift service | ||
| # Use separate stop and start steps to avoid a race condition with | ||
| # restart where we can't tell when it is down and so we can't tell | ||
| # whether we should count it as back up yet. Forcing the service | ||
| # down, then back up, ensures that when we do things like check | ||
| # systemd settings for the running process we get the values we | ||
| # expect without worrying about the race. Any test that modifies | ||
| # the MicroShift configuration file will be more reliable as a | ||
| # result. | ||
| Systemctl With Retry stop microshift.service | ||
| Sleep 10 seconds | ||
|
dhellmann marked this conversation as resolved.
Outdated
|
||
| Systemctl With Retry start microshift.service | ||
| Wait For MicroShift | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| *** Settings *** | ||
| Documentation Keywords for interacting with systemd | ||
|
|
||
| Library Process | ||
| Library String | ||
| Resource ../resources/microshift-host.resource | ||
|
|
||
|
|
||
| *** Keywords *** | ||
| Get Systemd Setting | ||
| [Documentation] Fetch one setting from systemd for the named unit. | ||
| ... Take care to get the unit_name value _exactly_ right, or | ||
| ... systemd will report a default value without reporting any error | ||
| ... or warning. | ||
| [Arguments] ${unit_name} ${property} | ||
|
|
||
| ${stdout} ${rc}= Execute Command | ||
| ... systemctl show --property=${property} --value ${unit_name} | ||
| ... sudo=True return_rc=True | ||
| Should Be Equal As Integers ${rc} 0 | ||
| Should Not Be Empty ${stdout} | ||
|
|
||
| ${result}= Strip String ${stdout} | ||
| RETURN ${result} | ||
|
|
||
| Systemctl # robocop: disable=too-long-keyword | ||
| [Documentation] Run a systemctl command on the microshift host. | ||
| ... The intent is to start, stop, or restart a service. Other | ||
| ... commands should be implemented separately. When the verb is | ||
| ... "start" or "restart", this keyword will wait for the unit | ||
| ... to report that it is "running". When the verb is "stop", this | ||
| ... keyword will wait for the unit to report that it is "dead". | ||
| [Arguments] ${verb} ${unit_name} | ||
|
|
||
| IF "${verb}" in {"restart", "start"} | ||
| ${state}= Set Variable running | ||
| ELSE | ||
| ${state}= Set Variable dead | ||
| END | ||
|
|
||
| ${stdout} ${stderr} ${rc}= Execute Command | ||
| ... systemctl ${verb} ${unit_name} | ||
| ... sudo=True | ||
| ... return_stdout=True | ||
| ... return_stderr=True | ||
| ... return_rc=True | ||
| IF ${rc} != 0 | ||
| ${status_text}= Execute Command | ||
| ... systemctl status ${unit_name} | ||
| ... sudo=True | ||
| ... return_stdout=True | ||
| ${log_text}= Execute Command | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
| ... journalctl -u ${unit_name} -o short | tail -n 100 | ||
| ... sudo=True | ||
| ... return_stdout=True | ||
| END | ||
| Should Be Equal As Integers 0 ${rc} | ||
|
|
||
| # It takes a bit for systemd to respond, and if we check too soon | ||
| # then it looks like microshift is up, even though it is about to | ||
| # be restarted. | ||
| Sleep 5s | ||
|
|
||
| Wait Until Keyword Succeeds | ||
| ... 10x | ||
| ... 10s | ||
| ... Execute Command | ||
| ... [ $(systemctl show -p SubState --value ${unit_name}) = ${state} ] | ||
| ... timeout=10s return_stdout=True return_stderr=True | ||
|
|
||
| Systemctl With Retry | ||
| [Documentation] Run Systemctl keyword but retry 10 times | ||
| [Arguments] ${verb} ${unit_name} | ||
| Wait Until Keyword Succeeds | ||
| ... 10x | ||
| ... 10s | ||
| ... Systemctl ${verb} ${unit_name} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| *** Settings *** | ||
| Documentation Tests related to how etcd is managed | ||
|
|
||
| Resource ../resources/common.resource | ||
| Resource ../resources/systemd.resource | ||
| Resource ../resources/microshift-config.resource | ||
| Resource ../resources/microshift-process.resource | ||
| Library Collections | ||
|
|
||
| Suite Setup Setup | ||
| Suite Teardown Teardown | ||
|
|
||
| Test Tags etcd configuration restart slow | ||
|
|
||
|
|
||
| *** Variables *** | ||
| ${ETCD_SYSTEMD_UNIT} microshift-etcd.scope | ||
| ${MEMLIMIT128} SEPARATOR=\n | ||
| ... --- | ||
| ... etcd: | ||
| ... \ \ memoryLimitMB: 128 | ||
| ${MEMLIMIT0} SEPARATOR=\n | ||
| ... --- | ||
| ... etcd: | ||
| ... \ \ memoryLimitMB: 0 | ||
|
|
||
|
|
||
| *** Test Cases *** | ||
| Set MemoryHigh Limit Unlimited | ||
| [Documentation] The default configuration should not limit RAM | ||
| ... | ||
| ... Since we cannot assume that the default configuration file is | ||
| ... being used, the test explicitly configures a '0' limit, which | ||
| ... is equivalent to not having any configuration at all. | ||
| [Setup] Setup With Custom Config ${MEMLIMIT0} | ||
| Expect MemoryHigh infinity | ||
|
|
||
| Set MemoryHigh Limit 128MB | ||
| [Documentation] Set the memory limit for etcd to 128MB and ensure it takes effect | ||
| [Setup] Setup With Custom Config ${MEMLIMIT128} | ||
| # Expecting the setting to be 128 * 1024 * 1024 | ||
| Expect MemoryHigh 134217728 | ||
| [Teardown] Restore Default Config | ||
|
|
||
|
|
||
| *** Keywords *** | ||
| Setup | ||
| [Documentation] Test suite setup | ||
| Check Required Env Variables | ||
| Login MicroShift Host | ||
| Setup Kubeconfig # for readiness checks | ||
| Save Default MicroShift Config | ||
|
|
||
| Teardown | ||
| [Documentation] Test suite teardown | ||
| Restore Default Config | ||
| Logout MicroShift Host | ||
| Remove Kubeconfig | ||
|
|
||
| Restore Default Config | ||
| [Documentation] Remove any custom config and restart MicroShift | ||
| Restore Default MicroShift Config | ||
| Restart MicroShift | ||
|
|
||
| Setup With Custom Config | ||
| [Documentation] Install a custom config and restart MicroShift | ||
| [Arguments] ${config_content} | ||
| ${merged}= Extend MicroShift Config ${config_content} | ||
| Upload MicroShift Config ${merged} | ||
| Restart MicroShift | ||
|
|
||
| Expect MemoryHigh | ||
| [Documentation] Verify that the MemoryHigh setting for etcd matches the expected value | ||
| [Arguments] ${expected} | ||
| ${actual}= Get Systemd Setting microshift-etcd.scope MemoryHigh | ||
| # Using integer comparison is complicated here because sometimes | ||
| # the returned or expected value is 'infinity'. | ||
| Should Be Equal ${expected} ${actual} |
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.
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.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.
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.