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

Manage swap partition as a hard requirement #156

Merged

Conversation

bogdando
Copy link
Contributor

@bogdando bogdando commented May 18, 2023

Add swap partition management into the bootstrap role

The swap file/partition management logic is carried over from the existing EDPM pre-adoption implementation, but moved out of the nova compute specific scope into generic tasks for EDPM.

Depends-On: https://review.rdoproject.org/r/c/rdo-jobs/+/48683

Closes: OSPRH-133

@bogdando
Copy link
Contributor Author

bogdando commented May 18, 2023

The molecule testing is blocked as we cannot use swapon [0] from podman containers, we'd need a delegated driver for that, likely. Any hints to make this unblocked, @raukadah perchance?

UPDATE:
#154 zuul job example
we can move the job to zuul job and there we can use the delegated driver

[0]

Configure swap file] ***************************\nfatal: [instance]: FAILED! => {\"changed\": true, \"cmd\": \"#!/bin/bash\\nset -eu\\nif [ ! -f /swap ]; then\\n  dd if=/dev/zero of=/swap count=4096 bs=1M\\n  chmod 0600 /swap\\n  mkswap /swap\\n  swapon /swap\\n  if ! grep -qE \\\"/swap\\\\b\\\" /etc/fstab; then\\n    echo \\\"/swap swap swap defaults 0 0\\\" >> /etc/fstab\\n  fi\\nfi\\n\", \"delta\": \"0:00:20.914521\", \"end\": \"2023-05-18 11:57:16.365692\", \"msg\": \"non-zero return code\", \"rc\": 255, \"start\": \"2023-05-18 11:56:55.451171\", \"stderr\": \"4096+0 records in\\n4096+0 records out\\n4294967296 bytes (4.3 GB, 4.0 GiB) copied, 16.0297 s, 268 MB/s\\nswapon: /swap: swapon failed: Operation not permitted\"

@bogdando
Copy link
Contributor Author

bogdando commented May 19, 2023

the rdo-project job is failed, probably by an unrelated reason [0], because I can see the swap task completed there [1]

[0]

TASK [osp.edpm.edpm_ovn : Check if OVS Manager already exists] *****************
fatal: [38.102.83.191]: FAILED! => {"changed": true, "cmd": "ovs-vsctl show | grep -q \"Manager\"\n", "delta": "0:00:00.010949", "end": "2023-05-18 13:58:38.366337", "msg": "non-zero return code", "rc": 1, "start": "2023-05-18 13:58:38.355388", "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": []}

[1] https://logserver.rdoproject.org/56/156/814b11000ae0869a6e1559bee243ec11f332ff28/github-check/edpm-ansible-github-rdo-integration-centos-8-crc-singlenode-centos-9-external-compute/79eb0cc/controller/controller/pod/deploy-external-dataplane-compute-n4zc8-logs.txt

@bogdando
Copy link
Contributor Author

/test rdoproject.org/github-check

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 19, 2023

@bogdando: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pre-commit-test

Use /test all to run all jobs.

In response to this:

/test rdoproject.org/github-check

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.

@bogdando
Copy link
Contributor Author

check-rdo

1 similar comment
@bogdando
Copy link
Contributor Author

check-rdo

@jpodivin
Copy link
Contributor

Just to clarify. The molecule in podman seems to work with this patch after all https://github.com/openstack-k8s-operators/edpm-ansible/actions/runs/5014407281/jobs/8988616358?pr=156 . Do you still want to replace it with delegated driver?

@bogdando
Copy link
Contributor Author

Just to clarify. The molecule in podman seems to work with this patch after all https://github.com/openstack-k8s-operators/edpm-ansible/actions/runs/5014407281/jobs/8988616358?pr=156 . Do you still want to replace it with delegated driver?

no, please let's use the mocked swapon command instead. So this is good to go as is!

@bogdando
Copy link
Contributor Author

PTAL folks

roles/edpm_bootstrap/tasks/bootstrap.yml Outdated Show resolved Hide resolved
roles/edpm_bootstrap/defaults/main.yml Show resolved Hide resolved
roles/edpm_bootstrap/defaults/main.yml Outdated Show resolved Hide resolved
roles/edpm_bootstrap/tasks/bootstrap.yml Outdated Show resolved Hide resolved
@bogdando
Copy link
Contributor Author

bogdando commented Jun 7, 2023

@softwarefactory-project-zuul
Copy link

Add swap partition management task into the bootstrap role

The swap file/partition management logic is carried over from the
existing EDPM pre-adoption implementation, but moved out of the
nova compute specific scope into generic tasks for EDPM.

Mock the swapon command as it fails for the podman molecule driver.

Signed-off-by: Bohdan Dobrelia <bdobreli@redhat.com>
@softwarefactory-project-zuul
Copy link

Copy link
Contributor

@bshephar bshephar left a comment

Choose a reason for hiding this comment

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

I can see this role has some inconsistencies with FQCN usage. But I think we should probably aim for using the FQCN's with these new PR's. We can clean up the rest of the role later.

I see you mentioned in the comments that you're just porting a pre-existing thing to edpm-ansible in order to unblock some work. So I'm happy to merge this one and address the comments I made in a follow up PR if you prefer.

roles/edpm_bootstrap/tasks/swap.yml Show resolved Hide resolved
roles/edpm_bootstrap/tasks/swap.yml Show resolved Hide resolved
roles/edpm_bootstrap/tasks/swap.yml Show resolved Hide resolved
@bogdando
Copy link
Contributor Author

/recheck

@bogdando
Copy link
Contributor Author

@kajinamit PTAL

Copy link
Contributor

@kajinamit kajinamit left a comment

Choose a reason for hiding this comment

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

I don't know why github doesn't remove my change request.

@openshift-ci openshift-ci bot added the lgtm label Jun 22, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bogdando, kajinamit

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rabi
Copy link
Contributor

rabi commented Jun 22, 2023

recheck

@softwarefactory-project-zuul
Copy link

@rebtoor
Copy link
Contributor

rebtoor commented Jun 27, 2023

recheck

@openshift-merge-robot openshift-merge-robot merged commit 465d20b into openstack-k8s-operators:main Jun 27, 2023
23 checks passed
@bogdando bogdando deleted the swap_tasks branch September 13, 2023 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants