-
Notifications
You must be signed in to change notification settings - Fork 5
docs: add VMO cluster deployment DOC-1358 #46
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
base: main
Are you sure you want to change the base?
Conversation
c127eb5
to
d1ac9fe
Compare
docs: add creation of virtual machine docs: add test cases DOC-1358 docs: add readme DOC-1358
d1ac9fe
to
4698a97
Compare
There was a problem hiding this 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 🚀
There was a problem hiding this 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.
terraform/vmo-cluster/clusters.tf
Outdated
instance_type { | ||
min_cpu = var.ctl-node-min-cpu | ||
min_memory_mb = var.ctl-node-min-memory-mb | ||
} |
There was a problem hiding this comment.
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
.
terraform/vmo-cluster/clusters.tf
Outdated
instance_type { | ||
min_cpu = var.wrk-node-min-cpu | ||
min_memory_mb = var.wrk-node-min-memory-mb | ||
} |
There was a problem hiding this comment.
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
terraform/vmo-cluster/data.tf
Outdated
data "spectrocloud_cluster" "maas_vmo_cluster" { | ||
count = var.deploy-maas-vm ? 1 : 0 | ||
depends_on = [spectrocloud_cluster_maas.maas-cluster] | ||
name = "vmo-cluster-maas" |
There was a problem hiding this comment.
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.
terraform/vmo-cluster/inputs.tf
Outdated
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete these empty lines?
terraform/vmo-cluster/inputs.tf
Outdated
|
||
variable "cluster-profile-type" { | ||
type = string | ||
description = "The name of the PCG that will be used to deploy the cluster." |
There was a problem hiding this comment.
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.
terraform/vmo-cluster/tests/virtual-machines-replace-values.tftest.hcl
Outdated
Show resolved
Hide resolved
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 |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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?
There was a problem hiding this 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 {} |
There was a problem hiding this comment.
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?
Describe the Change
This PR adds the VMO cluster deployment and VM creation example.
Review Changes
🎫 DOC-1358