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

Add move_vm_from_on_prem_to_aws playbook and corresponding roles #95

Merged

Conversation

alinabuzachis
Copy link
Contributor

@alinabuzachis alinabuzachis commented Sep 12, 2023

Add move_vm_from_on_prem_to_aws playbook and corresponding roles

Requires ansible-collections/amazon.aws#1723

@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch 2 times, most recently from b6597d6 to 7fa9b60 Compare September 19, 2023 14:11
@alinabuzachis alinabuzachis changed the title [WIP] Add move_vm_from_on_prem_to_aws role Add move_vm_from_on_prem_to_aws role Sep 19, 2023
playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
roles/clone_on_prem_vm/README.md Outdated Show resolved Hide resolved
roles/clone_on_prem_vm/tasks/main.yml Outdated Show resolved Hide resolved
roles/clone_on_prem_vm/tasks/main.yml Outdated Show resolved Hide resolved
roles/import_image_and_run_aws_instance/README.md Outdated Show resolved Hide resolved
roles/import_image_and_run_aws_instance/README.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Was this file intended to be checked in? These values seem very specific to a local testing environment.

Copy link
Contributor Author

@alinabuzachis alinabuzachis Sep 21, 2023

Choose a reason for hiding this comment

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

Yes, it was intended to show some examples, but let me know if I have to remove it in case.

- name: Import 'cloud.aws_ops.import_image_and_run_aws_instance' role
ansible.builtin.import_role:
name: cloud.aws_ops.import_image_and_run_aws_instance
vars:
Copy link
Member

Choose a reason for hiding this comment

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

It looks like there are some variables missing here. How does the user set the security groups, keypair, subnet, etc.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not set those because they are optional, but if we want add all the variables that the role accepts I can do that.

playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
* **keypair_name** (str): (Optional) The name of the SSH access key to assign to the EC2 instance. It must exist in the region the instance is created. If not set, your default AWS account keypair will be used.
* **security_groups** (list): (Optional) A list of security group IDs or names to assiciate to the EC2 instance.
* **vpc_subnet_id** (str): (Optional) The subnet ID in which to launch the EC2 instance instance (VPC). If none is provided, M(amazon.aws.ec2_instance) will chose the default zone of the default VPC.
* **instance_volumes** (dict): (Optional) A dictionary of a block device mappings, by default this will always use the AMI root device so the **instance_volumes** option is primarily for adding more storage. A mapping contains the (optional) keys _device_name_, _ebs.volume_type_, _ebs.volume_size_, _ebs.kms_key_id_, _ebs.iops_, and _ebs.delete_on_termination_.
Copy link
Member

Choose a reason for hiding this comment

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

This would benefit from having the same structure as you use for the kvm_host var.

@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch from f15ca51 to 948f1b8 Compare September 21, 2023 18:47
@alinabuzachis alinabuzachis changed the title Add move_vm_from_on_prem_to_aws role Add move_vm_from_on_prem_to_aws playbook and corresponding roles Sep 21, 2023
@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch 5 times, most recently from 446a21c to 08fd92f Compare September 22, 2023 11:02
Copy link
Member

@gravesm gravesm left a comment

Choose a reason for hiding this comment

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

One small thing, otherwise looks good.

playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
roles/clone_on_prem_vm/README.md Outdated Show resolved Hide resolved
roles/clone_on_prem_vm/README.md Outdated Show resolved Hide resolved
roles/clone_on_prem_vm/tasks/main.yml Outdated Show resolved Hide resolved
roles/import_image_and_run_aws_instance/README.md Outdated Show resolved Hide resolved
roles/import_image_and_run_aws_instance/tasks/main.yml Outdated Show resolved Hide resolved
roles/import_image_and_run_aws_instance/tasks/main.yml Outdated Show resolved Hide resolved
roles/import_image_and_run_aws_instance/tasks/main.yml Outdated Show resolved Hide resolved
@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch 2 times, most recently from 14afa21 to 72ef7a0 Compare October 5, 2023 14:31
* **ansible_ssh_private_key_file** This variable specifies the path to the SSH private key file that Ansible should use for authentication when connecting to the host.
* **groups** This variable enabled you to assign the newly added host to one or more groups in the inventory.

### Needed for the cloud.aws_ops.clone_on_prem_vm role
Copy link
Contributor

Choose a reason for hiding this comment

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

You should reference here the file roles/clone_on_prem_vm/REAME.md to avoid duplicate and for the playbook and role to be synced if there is an update on the role documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done that. Let me know please if that works!

* **clone_on_prem_vm_overwrite** (bool): (Optional) Whether to overwrite or not an already existing on prem VM clone. Default: true.
* **clone_on_prem_vm_local_image_path** (str): (Optional) The path where you would like to save the image. If the path does not exists on localhost, the role will create it. If this parameter is not set, the role will save the image in a _~/tmp_ folder.

### Needed for the cloud.aws_ops.import_image_and_run_aws_instance role
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above


## Playbook Variables

* **kvm_host** (dict): Information about the host running the KVM hypervisr that are dynamically added to the inventory.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather prefer having this describe into an inventory file that will be passed when running ansible playbook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done that. Let me know please if that works!

region: "{{ aws_region | default('us-east-1') }}"

tasks:
- name: Add host to inventory
Copy link
Contributor

Choose a reason for hiding this comment

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

would prefer to move this into an inventory file

ansible_ssh_private_key_file: ~/.ssh/id_rsa.pub

tasks:
- name: Add host to inventory
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, should be better to define this in an inventory and run the playbook on all hosts

@alinabuzachis alinabuzachis requested a review from abikouo October 9, 2023 13:49
Copy link
Collaborator

@hakbailey hakbailey left a comment

Choose a reason for hiding this comment

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

Just a couple more small typos. otherwise this looks good!

playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
playbooks/move_vm_from_on_prem_to_aws/README.md Outdated Show resolved Hide resolved
@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch from d3340ce to baa2a99 Compare October 10, 2023 14:20
@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch from bfa24a5 to bd4088f Compare October 18, 2023 09:20

All the variables defined in section ``Playbook Variables`` can be defined inside the ``vars.yml`` file.

Create a ``playbook.ym`` file like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Create a ``playbook.ym`` file like this:
Create a ``playbook.yml`` file like this:


All the variables defined in section ``Playbook Variables`` can be defined inside the ``vars.yml`` file.

Create a ``playbook.ym`` file like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Create a ``playbook.ym`` file like this:
Create a ``playbook.yml`` file like this:

groups: mygroup
```

All the variables defined in section ``Playbook Variables`` can be defined inside the ``vars.yml`` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

I apologize if I've missed it, but could you please point me to where the playbook variables section is located?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here https://github.com/redhat-cop/cloud.aws_ops/pull/95/files#diff-3a382ac08ed4e75c0e95cbbabd59acfc9e5c8d543f4882cb588ee93f0ad0029fR10. To avoid content duplication I referenced the variable roles which are also the ones required by the playbook.

@alinabuzachis alinabuzachis force-pushed the move_vm_from_on_prem_to_aws branch from 73f8479 to fd9a487 Compare October 24, 2023 19:37
@alinabuzachis alinabuzachis merged commit f24c6f3 into redhat-cop:main Oct 25, 2023
19 of 20 checks passed
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.

5 participants