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.

Sign up for GitHub

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

Jump to bottom

Disable swap space on nodes at install and upgrade #3884

Merged
merged 1 commit into from
Apr 13, 2017
Merged

Disable swap space on nodes at install and upgrade #3884

merged 1 commit into from
Apr 13, 2017

Conversation

mtnbikenc
Copy link
Member

@mtnbikenc mtnbikenc self-assigned this Apr 7, 2017
@mtnbikenc mtnbikenc requested a review from sdodson April 7, 2017 18:13
@mtnbikenc
Copy link
Member Author

aos-ci-test

@openshift-bot
Copy link

success: aos-ci-jenkins/OS_unit_tests for 68b9cdf (logs)

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_containerized for 68b9cdf (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for 68b9cdf (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for 68b9cdf (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 68b9cdf (logs)

@mtnbikenc mtnbikenc requested a review from tbielawa April 10, 2017 12:42
@sdodson
Copy link
Member

sdodson commented Apr 10, 2017

How much of a challenge do you think it'd be to add a task that checks to ensure that we have enough free memory versus swap in use before doing this and perform that as a pre-upgrade check? I believe the formula we'd use is to grab MemFree, SwapTotal, SwapFree from /proc/meminfo and abort if (MemFree < (SwapTotal - SwapFree) )

The output of free may be easier too.

@sdodson
Copy link
Member

sdodson commented Apr 10, 2017

That said, if we disable swap while the node is evacuated the memory usage will be vastly different than what it is when prior to performing the upgrade and it's difficult to determine how evacuation will affect usage.

sdodson
sdodson requested changes Apr 10, 2017
View reviewed changes
roles/openshift_node/tasks/main.yml Outdated
@@ -34,6 +34,32 @@
dns_ip: "{{ openshift_dns_ip | default(none) | get_dns_ip(hostvars[inventory_hostname])}}"
env_vars: "{{ openshift_node_env_vars | default(None) }}"

# https://docs.openshift.com/container-platform/3.4/admin_guide/overcommit.html#disabling-swap-memory
- name: Check for swap usage
command: grep swap /etc/fstab
Copy link
Member

Choose a reason for hiding this comment

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

grep -v "^#" /etc/fstab | grep swap or something better to avoid false positives on commented lines?

@mtnbikenc
Copy link
Member Author

Updated grep command to ignore commented lines.

@mtnbikenc
Copy link
Member Author

aos-ci-test

sdodson
sdodson approved these changes Apr 10, 2017
View reviewed changes
Copy link
Member

@sdodson sdodson left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_containerized for a55f47d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for a55f47d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for a55f47d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for a55f47d (logs)

@sdodson
Copy link
Member

sdodson commented Apr 11, 2017

aos-ci-test

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.5_containerized for a55f47d (logs)

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_containerized for a55f47d (logs)

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.5_NOT_containerized for a55f47d (logs)

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_NOT_containerized for a55f47d (logs)

@mtnbikenc
Copy link
Member Author

aos-ci-test

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_containerized for a55f47d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for a55f47d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for a55f47d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for a55f47d (logs)

@mtnbikenc
Copy link
Member Author

[merge]

@openshift-bot
Copy link

[test]ing while waiting on the merge queue

@mtnbikenc
Copy link
Member Author

aos-ci-test

@openshift-bot
Copy link

Evaluated for openshift ansible test up to 38e0c9c

@openshift-bot
Copy link

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/25/) (Base Commit: 2942b03)

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_containerized for 38e0c9c (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for 38e0c9c (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 38e0c9c (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for 38e0c9c (logs)

@mtnbikenc
Copy link
Member Author

Flake openshift/origin#13271

@mtnbikenc
Copy link
Member Author

[merge]

@mtnbikenc
Copy link
Member Author

Flake openshift/origin#12629 Downward API volume
Flake openshift/origin#13705 Dumping container logs
re[merge]

@openshift-bot
Copy link

Evaluated for openshift ansible merge up to 38e0c9c

@openshift-bot
Copy link

openshift-bot commented Apr 13, 2017 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/222/) (Base Commit: e36b53c)

@openshift-bot openshift-bot merged commit 6591b6f into openshift:master Apr 13, 2017
@mtnbikenc mtnbikenc deleted the disable-swap branch April 13, 2017 18:49
@rhcarvalho
Copy link
Contributor

How much of a challenge do you think it'd be to add a task that checks to ensure that we have enough free memory versus swap in use before doing this and perform that as a pre-upgrade check? I believe the formula we'd use is to grab MemFree, SwapTotal, SwapFree from /proc/meminfo and abort if (MemFree < (SwapTotal - SwapFree) )

The output of free may be easier too.

@mtnbikenc @sdodson the builtin Ansible "setup" module provides memory-related facts such that we may not need to call free ourselves. We've been using it in #3518.
In particular, we look at ansible_memtotal_mb:
https://github.com/openshift/openshift-ansible/pull/3518/files#diff-7cb3c3806a4642e46d907eeed0c8873eR28

@sdodson
Copy link
Member

sdodson commented Apr 18, 2017

So, based on that if we choose to add it, if ( ansible_memfree_mb - ansible_memory_mb.swap.used > 0 ) we're good to go? Anyway, given we're performing this either at install time or during upgrade when the node is already drained I'm relatively confident that we won't run into problems.

        "ansible_memory_mb": {
            "nocache": {
                "free": 3507, 
                "used": 284
            }, 
            "real": {
                "free": 2012, 
                "total": 3791, 
                "used": 1779
            }, 
            "swap": {
                "cached": 0, 
                "free": 0, 
                "total": 0, 
                "used": 0
            }
        }, 
        "ansible_memtotal_mb": 3791,

@lukas-vlcek lukas-vlcek mentioned this pull request Apr 19, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdodson sdodson sdodson approved these changes

@tbielawa tbielawa Awaiting requested review from tbielawa

Assignees

@mtnbikenc mtnbikenc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mtnbikenc @openshift-bot @sdodson @rhcarvalho