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

feat: provisioning script #335

Merged
merged 1 commit into from
Dec 22, 2021
Merged

Conversation

Callisto13
Copy link
Member

@Callisto13 Callisto13 commented Dec 21, 2021

provision.sh tool

Drawn a lot from firecracker's one of these, but not as much as i expected as their bits made my shellcheck yell at me a lot

There are docs included in this PR, but here is the help:

root@callisto:~# ./provision.sh -h
usage: ./provision.sh <COMMAND> <OPTIONS>

Script to provision hosts for running flintlock microvms

COMMANDS:

  all                    Complete setup for production ready host. Component versions
                         can be configured by setting the FLINTLOCK, CONTAINERD and FIRECRACKER
                         environment variables.
    OPTIONS:
      -y                 Autoapprove all prompts (danger)
      --skip-apt, -s     Skip installation of apt packages
      --thinpool, -t     Name of thinpool to create (default: flintlock or flintlock-dev)
      --disk, -d         Name blank unpartioned disk to use for direct lvm thinpool (ignored if --dev set)
      --dev              Set up development environment. Loop thinpools will be created.

  apt                    Install all apt packages required by flintlock

  firecracker            Install firecracker from feature branch
    OPTIONS:
      --version, -v      Version to install (default: latest)

  containerd             Install, configure and start containerd service
    OPTIONS:
      --version, -v      Version to install (default: latest)
      --thinpool, -t     Name of thinpool to include in config toml (default: flintlock-thinpool)
      --dev              Set up development environment. Containerd will keep state under 'dev' tagged paths.

  flintlock              Install and start flintlockd service
    OPTIONS:
      --version, -v      Version to install (default: latest)
      --dev              Assumes containerd has been provisioned in a dev environment

  direct_lvm             Set up direct_lvm thinpool
    OPTIONS:
      -y                 Autoapprove all prompts (danger)
      --thinpool, -t     Name of thinpool to create (default: flintlock)
      --disk, -d         Name blank unpartioned disk to use for direct lvm thinpool
      --skip-apt, -s     Skip installation of apt packages

  devpool                Set up loop device thinpool (development environments)
    OPTIONS:
      --thinpool, -t     Name of thinpool to create (default: flintlock-dev)

And an action shot:

Screenshot 2021-12-21 at 16 33 57

@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2021

Codecov Report

Merging #335 (f45c3ff) into main (aeec4d9) will decrease coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #335      +/-   ##
==========================================
- Coverage   58.29%   58.13%   -0.16%     
==========================================
  Files          51       51              
  Lines        2503     2503              
==========================================
- Hits         1459     1455       -4     
- Misses        929      932       +3     
- Partials      115      116       +1     
Impacted Files Coverage Δ
pkg/queue/queue.go 93.93% <0.00%> (-6.07%) ⬇️
infrastructure/containerd/event_service.go 56.14% <0.00%> (-3.51%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aeec4d9...f45c3ff. Read the comment docs.

@Callisto13 Callisto13 marked this pull request as ready for review December 21, 2021 16:36
@Callisto13 Callisto13 force-pushed the provision branch 2 times, most recently from 3eede98 to e4e203c Compare December 22, 2021 10:50
Copy link
Contributor

@yitsushi yitsushi left a comment

Choose a reason for hiding this comment

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

I'm about half-way (at ## CONTAINERD) and have a few comments already, so i'll just submit them now and continue with the rest.

hack/scripts/provision.sh Outdated Show resolved Hide resolved
Comment on lines +156 to +160
local repo_name="$1"
local tag="$2"
local bin="$3"
echo "https://github.com/$repo_name/releases/download/$tag/$bin"
Copy link
Contributor

Choose a reason for hiding this comment

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

I really love all this variables names to document what are those arguments and
not just https://github.com/${1}/.....

Comment on lines +193 to +195
local url=$(curl -sL "$latest_url" | awk -F'"' '/tag_name/ {printf $4}')
echo "$url"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not:

Suggested change
local url=$(curl -sL "$latest_url" | awk -F'"' '/tag_name/ {printf $4}')
echo "$url"
curl -sL "$latest_url" | awk -F'"' '/tag_name/ {printf $4}'

Copy link
Member Author

Choose a reason for hiding this comment

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

i wanted to make the return more explicit, but yes it is unnecessary

Comment on lines +201 to +204
tag=$(git ls-remote --tags --sort="v:refname" "https://github.com/$repo_name" |
tail -n 1 | sed 's/.*\///; s/\^{}//')
echo "$tag"
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 latest_release_tag.

hack/scripts/provision.sh Outdated Show resolved Hide resolved
hack/scripts/provision.sh Outdated Show resolved Hide resolved
hack/scripts/provision.sh Outdated Show resolved Hide resolved
hack/scripts/provision.sh Show resolved Hide resolved
hack/scripts/provision.sh Outdated Show resolved Hide resolved
Comment on lines +491 to +501
lvcreate -q --wipesignatures y -n thinpool "$volume_group" -l 95%VG || die "Failed to create logical volume for thinpool data"
say "Created logical volume for $volume_group thinpool data"

lvcreate -q --wipesignatures y -n thinpoolmeta "$thinpool" -l 1%VG || die "Failed to create logical volume for thinpool metadata"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm.. That 95% and 1% combo seems strange.

  1. where is the remaining 4%
  2. the METADATA_SPARSE_SIZE="10G" DATA_SPARSE_SIZE="100G" pair has a 1:10 ratio.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's even more fishy, we have fix size for spare files, but we use dynamic volume size with lvm.

If the disk full size is less than $((DATA_SPARSE_SIZE + METADATA_SPARSE_SIZE)) we should fail before we even try to erase it.

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm, i see DATA_SPARSE_SIZE and METADATA_SPARSE_SIZE are used for dev only without lvm.

(leave this thread here to record my confusion 😆 )

Copy link
Member Author

Choose a reason for hiding this comment

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

i copied all the devpool stuff from our existing script, did not actually read any of it 😁

hack/scripts/provision.sh Outdated Show resolved Hide resolved
yitsushi
yitsushi previously approved these changes Dec 22, 2021
@Callisto13
Copy link
Member Author

😬 sorry @yitsushi I had a mistake in the docs

@Callisto13 Callisto13 merged commit 898d1d8 into liquidmetal-dev:main Dec 22, 2021
@Callisto13 Callisto13 deleted the provision branch December 22, 2021 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants