-
Notifications
You must be signed in to change notification settings - Fork 70
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
Re-enable swap for bootc #873
Re-enable swap for bootc #873
Conversation
roles/edpm_bootstrap/tasks/swap.yml
Outdated
- not edpm_bootstrap_swap_partition_enabled|bool | ||
- edpm_bootstrap_swap_size_megabytes|int > 0 | ||
become: true | ||
block: | ||
- name: Set swap var to use (bootc) | ||
set_fact: | ||
edpm_bootstrap_swap_path_value: "{{ edpm_bootstrap_swap_path_bootc }}" |
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.
Didn't want to do this with Jinja2 instead of the two conditional tasks?
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.
lgtm
The default path for a swapfile needs to be under a writeable location for bootc. Use /var/swap. The value of the existing variable, edpm_bootstrap_swap_path can't just be changed since there are already existing deployments, so a new variable, edpm_bootstrap_swap_path_bootc is added for bootc. An ansible task will select which variable to use based on the type of system. Signed-off-by: James Slagle <jslagle@redhat.com>
@@ -72,6 +72,9 @@ edpm_bootstrap_selinux_mode: enforcing | |||
# Swap management | |||
edpm_bootstrap_swap_size_megabytes: 1024 | |||
edpm_bootstrap_swap_path: /swap | |||
# swap path needs to be under /var on bootc, so a separate default value is | |||
# needed. | |||
edpm_bootstrap_swap_path_bootc: /var/swap |
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.
so other then the upgrade impact of needing to move the swap file
there is no reason we could not use /var/swap for all deployments.
we may want to consider doing that.
for example we could define new vars like this
edpm_bootstrap_legacy_swap_path: /swap
edpm_bootstrap_swap_path: /var/swap
then check if edpm_bootstrap_legacy_swap_path exists and edpm_bootstrap_swap_path does not.
if that is the case check fi there is enough space to allocate the second swap file, do swap on to the new location and then swappoff the old location and delete the legacy swap file.
this can be a followup pr and could be deferred to the next reboot also.
but that would give us a way to normalise the two flows.
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.
It would indeed be nice to bring consistency to both types of RHEL nodes. Just a small inconvenience during the transition
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bshephar, slagle The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2615417
into
openstack-k8s-operators:bootc
The default path for a swapfile needs to be under a writeable location
for bootc. Use /var/swap. The value of the existing variable,
edpm_bootstrap_swap_path can't just be changed since there are already
existing deployments, so a new variable, edpm_bootstrap_swap_path_bootc
is added for bootc. An ansible task will select which variable to use
based on the type of system.
Signed-off-by: James Slagle jslagle@redhat.com