WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@addetz
Copy link
Contributor

@addetz addetz commented Jan 13, 2025

Describe the Change

This PR adds the VMO cluster deployment and VM creation example.

Review Changes

🎫 DOC-1358

@addetz addetz requested a review from a team as a code owner January 13, 2025 15:59
@addetz addetz requested review from achuribooks, karl-cardenas-coding and lennessyy and removed request for a team January 13, 2025 15:59
@addetz addetz force-pushed the add-vmo-cluster-doc-1358 branch 10 times, most recently from c127eb5 to d1ac9fe Compare January 13, 2025 20:33
docs: add creation of virtual machine

docs: add test cases DOC-1358

docs: add readme DOC-1358
@addetz addetz force-pushed the add-vmo-cluster-doc-1358 branch from d1ac9fe to 4698a97 Compare January 13, 2025 20:34
Copy link
Contributor

@caroldelwing caroldelwing left a comment

Choose a reason for hiding this comment

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

Congratulations on making this configuration work 👏 This will really help our users.
Also the test cases cover everything we need 🚀

Copy link
Contributor Author

@addetz addetz 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 only comment on my own PR 😆 My main question is around the templating of the manifest files at this point.

Comment on lines 30 to 33
instance_type {
min_cpu = var.ctl-node-min-cpu
min_memory_mb = var.ctl-node-min-memory-mb
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggest name variables using the same format. Something like maas-control-node-min-cpu and maas-control-node-min-memory-mb.

Comment on lines 44 to 47
instance_type {
min_cpu = var.wrk-node-min-cpu
min_memory_mb = var.wrk-node-min-memory-mb
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment about the naming. maas-worker-node-min-cpu and maas-worker-node-min-memory-mb

data "spectrocloud_cluster" "maas_vmo_cluster" {
count = var.deploy-maas-vm ? 1 : 0
depends_on = [spectrocloud_cluster_maas.maas-cluster]
name = "vmo-cluster-maas"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to use the var as well now that you have provided one.

Comment on lines 34 to 36



Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delete these empty lines?


variable "cluster-profile-type" {
type = string
description = "The name of the PCG that will be used to deploy the cluster."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The description is copy-pasta.

Comment on lines 67 to 71
vm-storage-Gi = "50Gi" # Size of the disk (PVC) that your VM will have.
vm-cpu-cores = 2 # Number of CPU cores your VM will have.
vm-cpu-sockets = 1 # Number of physical CPU sockets the CPU cores should be spread over.
vm-cpu-threads = 2 # Number of CPU threads to use for the VM CPU
vm-memory-Gi = "4Gi" # Amount of RAM (memory) your VM will have
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should these be templated in the vmo-extras manifest?

cluster_context = data.spectrocloud_cluster.maas_vmo_cluster[0].context

#run_on_launch = true
run_strategy = "Halted"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this still need to be Halted?

@addetz addetz requested a review from caroldelwing June 17, 2025 13:44
Copy link
Contributor

@caroldelwing caroldelwing left a comment

Choose a reason for hiding this comment

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

PR reviewed :) Will approve it tomorrow after the suggestions are applied and we complete the tests.

guest = var.vm-memory-Gi
}

resources {}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this empty block required?

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.

3 participants