Skip to content

add lxd virtualization#140

Open
skuznetsov-klika wants to merge 6 commits into
mainfrom
10-17-feat-sparkles-adds-lxd-virtualization
Open

add lxd virtualization#140
skuznetsov-klika wants to merge 6 commits into
mainfrom
10-17-feat-sparkles-adds-lxd-virtualization

Conversation

@skuznetsov-klika

Copy link
Copy Markdown

No description provided.

@anthonyra anthonyra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's schedule a call to go over this and ensure it's functioning as expected since I can't personally run it on my local environment.

Comment on lines +10 to +14
molecule_yml:
platforms:
- name: instance
image: ubuntu:20.04
user: ansible

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If molecule is setup correctly this should be pulled in from

platforms:
- name: instance
image: ubuntu:noble

when using a command like molecule converge from the root directory

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

changed to noble

Comment on lines +17 to +26
- name: Ensure LXD is installed
ansible.builtin.command:
cmd: snap install lxd
become: true

- name: Initialize LXD (if not already initialized)
command: lxd init --auto
args:
creates: /var/lib/lxd/lxd.db
become: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the other molecule create.ymls, I tried to not do any of the installation steps here because depending on the developers work environment it could fail. For example, this would fail if they don't have snap installed. And would continue until they got it installed which might not be possible. They may also prefer a different package manager all together (homebrew, nix, etc)

This file should simply try to use lxd to create a VM and then update (dump) the molecule/shared/inventory/molecule_inventory. If LXD is not installed should simply fail with errors.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

updated lxd virtualization

mode: pull
server: https://cloud-images.ubuntu.com/releases
protocol: simplestreams
alias: "f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would recommend adding the following, to leverage the cloud-init that's used to stand-up vms in this collection (probably why you need to add so many preconditions)

        config:
          cloud-init.user-data: "{{ lookup('ansible.builtin.template', '../../../../depin/core/playbooks/templates/user-data.yml.j2 ') }}"
        wait_for_ipv4_addresses: true
        wait_for_container: true
        timeout: 300

Also the last couple lines here ensures the VM is online before preceeding

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

used

  vars:
    molecule_inventory:
      molecule:
        hosts: {}
    molecule_yml:
      platforms:
        - name: instance
          image_fingerprint: "noble"
          architecture: x86_64
          user: ansible

Comment on lines +6 to +11
vars:
molecule_yml:
platforms:
- name: instance
image: ubuntu:20.04
user: ansible

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be pulled in like in the create.yml file. This should be removed. If the image format doesn't work we should fix in the molecule.yml file not overriding it here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

used noble now - but noble uses the latest version, and after the update something may be broken.

args:
creates: /var/lib/lxd/lxd.db
become: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should add the following to ensure Pulumi ESC gets set in the cloud-init file.

    - name: Get ESC from Pulumi
      ansible.builtin.command: esc open deeep-network/dev/services --format dotenv
      changed_when: false
      register: pulumi_esc

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added without testing

- name: Create LXD containers if not exists using lxc launch
ansible.builtin.command: >
lxc launch ubuntu-cloud:{{ platform['image_fingerprint'] }} {{ platform['name'] }}
when: lxd_container_info.results[0].stdout == '[]'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The current code assumes the first result from lxd_container_info.results corresponds to the current platform in the loop, which may not be true when multiple platforms are defined. To properly match results with platforms, add index_var: platform_index to the loop_control block and reference lxd_container_info.results[platform_index].stdout instead.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

should be fixed at next phase

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