Skip to content

Conversation

jovial
Copy link

@jovial jovial commented Feb 24, 2023

A utility role to migrate VMs from hypervisors.

A utility role to migrate VMs from hypervisors.
@jovial jovial requested a review from a team as a code owner February 24, 2023 12:31
@jovial jovial force-pushed the feature/compute-drain branch from e8dbebd to a874cf3 Compare February 24, 2023 12:54
@jovial jovial force-pushed the feature/compute-drain branch from a874cf3 to 4e050f4 Compare February 24, 2023 12:59
@@ -0,0 +1,4 @@
---

nova_compute_drain_venv: "{{ virtualenv_path }}/openstack"
Copy link
Author

Choose a reason for hiding this comment

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

virtualenv_path is defined by kayobe. Should I add some default here? Something in the home directory maybe?

Choose a reason for hiding this comment

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

Yeah that would be a better default

---

nova_compute_drain_venv: "{{ virtualenv_path }}/openstack"
nova_compute_drain_delegate_host: "{{ groups['controllers'][0] }}"
Copy link
Author

Choose a reason for hiding this comment

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

Default doesn't make sense outside kayobe

Choose a reason for hiding this comment

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

localhost as a reasonable default?

Copy link

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

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

Could use a little README

@@ -0,0 +1,4 @@
---

nova_compute_drain_venv: "{{ virtualenv_path }}/openstack"

Choose a reason for hiding this comment

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

Yeah that would be a better default

---

nova_compute_drain_venv: "{{ virtualenv_path }}/openstack"
nova_compute_drain_delegate_host: "{{ groups['controllers'][0] }}"

Choose a reason for hiding this comment

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

localhost as a reasonable default?

virtualenv: "{{ nova_compute_drain_venv }}"
name:
- python-openstackclient
extra_args: "{% if pip_upper_constraints_file %}-c {{ pip_upper_constraints_file }}{% endif %}"

Choose a reason for hiding this comment

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

Suggested change
extra_args: "{% if pip_upper_constraints_file %}-c {{ pip_upper_constraints_file }}{% endif %}"
extra_args: "{% if nova_compute_drain_upper_constraints_file %}-c {{ nova_compute_drain_upper_constraints_file }}{% endif %}"

And add to defaults?

- pip
- setuptools
state: latest
virtualenv_command: /usr/bin/python3.6 -m venv

Choose a reason for hiding this comment

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

Suggested change
virtualenv_command: /usr/bin/python3.6 -m venv
virtualenv_command: /usr/bin/python3 -m venv

vars:
# NOTE: Without this, the delegate ansible_host variable will not
# be respected when using delegate_to.
ansible_host: "{{ hostvars[nova_compute_drain_delegate_host].ansible_host | default(nova_compute_drain_delegate_host) }}"

Choose a reason for hiding this comment

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

run_once: true?

@@ -0,0 +1,17 @@
---

- name: "Live migrate instance: {{ instance_uuid }}" # noqa no-changed-when

Choose a reason for hiding this comment

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

Add changed_when: true, remove noqa

---

- block:
- name: "Cold migrate instance: {{ instance_uuid }}" # noqa no-changed-when

Choose a reason for hiding this comment

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

Add changed_when: true, remove noqa

retries: 10
delay: 30

- name: "Confirm resize: {{ instance_uuid }}" # noqa no-changed-when

Choose a reason for hiding this comment

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

Add changed_when: true, remove noqa

vars:
ansible_host: "{{ hostvars[nova_compute_drain_delegate_host].ansible_host }}"
rescue:
- meta: noop # noqa unnamed-task

Choose a reason for hiding this comment

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

Could use a comment to explain why we're ignoring errors here.

@@ -0,0 +1,17 @@
---

Choose a reason for hiding this comment

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

Do we need to ignore errors here, or is it an async API that does not generally fail?

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

Successfully merging this pull request may close these issues.

2 participants