Skip to content
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

Adding a test that checks that collectd memory limit set to 512Mb #227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lnatapov
Copy link
Contributor

Adding a test that checks that collectd memory limit set to 512Mb

@@ -28,3 +28,11 @@
register: stat
changed_when: false
failed_when: stat.stdout_lines|length == 0

- name: TEST Check the collectd memory limit is set to 512Mb
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest renaming this to not include the specific value for the memory limit, in case it changes again.

i.e. "TEST Check the collectd memory limit"

Optional update


- name: TEST Check the collectd memory limit is set to 512Mb
ansible.builtin.shell: |
{{ container_bin }} inspect {{ collectd_container_name }} | grep 512 -c
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMHO, it would be better to search also for the right property in the container information.
Is the property set in HostConfig.Memory, HostConfig.memoryReservation or something else?

Searching for just 512 could cause a false pass if the characters appear in a hash, for example, such as what I got when querying an arbitrary container in my system:

$ podman inspect musing_goldberg | grep 512
                    "vcs-ref": "a995512a05037e3b60bbb1bf9fa6e394063131c3",

Copy link
Collaborator

Choose a reason for hiding this comment

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

The memory limit is represented in B.

A container run with a memory limit of 512M will actually fail this test.

Run a container and set the memory limit to 512M:

podman run --memory 512m --name not_collectd hello:latest

Then find the name
Check the memory limit.

podman inspect not_collectd | grep Memory
               "Memory": 536870912,
               "KernelMemory": 0,
               "MemoryReservation": 0,
               "MemorySwap": 1073741824,
               "MemorySwappiness": -1,

podman inspect not_collectd | grep 512
will return a result for the command used to run the container, possibly also some hashes.
If the container was mis-managed, or deployed with the wrong settings, or even with some other property that includes "512", this test will pass, without verifying anything. Or, it might return multiple lines of result.

Podman allows updating the properties of a container, including the memory limits, so even detecting the options used to deploy a container may not represent the properties.

I suggest either modifying the grep command, or using jq (which we might already use elsewhere in the tests.

For a modified grep command, try

{{ container_bin }} inspect {{ collectd_container_name }} | grep \"Memory\"

Which will match something like this:

               "Memory": 536870912,

The fail condition for the ansible task can be:

failed_when: "536870912" not in memory.stdout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants