Changes for Dedicated Host with Landing Zone VSI - Reg #777

Louies-Jhony · 2025-01-06T15:03:24Z

Description

Release required?

No release
Patch release (x.x.X)
Minor release (x.X.x)
Major release (X.x.x)

Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

If relevant, a test for the change is included or updated with this PR.
If relevant, documentation for the change is included or updated with this PR.

For mergers

Use a conventional commit message to set the release level. Follow the guidelines.
Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
Use the Squash and merge option.

Louies-Jhony · 2025-01-06T15:03:51Z

/run pipeline

Louies-Jhony · 2025-01-07T01:18:11Z

/run pipeline

Louies-Jhony · 2025-01-07T02:25:12Z

/run pipeline

Louies-Jhony · 2025-01-07T05:23:31Z

/run pipeline

Louies-Jhony · 2025-01-09T04:41:45Z

/run pipeline

Louies-Jhony · 2025-01-09T06:26:10Z

/run pipeline

Louies-Jhony · 2025-01-09T10:40:58Z

/run pipeline

Louies-Jhony · 2025-01-13T17:45:28Z

/run pipeline

Louies-Jhony · 2025-01-14T00:25:16Z

/run pipeline

Louies-Jhony · 2025-01-14T06:10:13Z

/run pipeline

Louies-Jhony · 2025-01-14T06:51:45Z

/run pipeline

Louies-Jhony · 2025-01-14T11:10:35Z

/run pipeline

ocofaigh · 2025-01-14T11:59:20Z

variables.tf

+  default     = null
+  description = "ID of the dedicated host for hosting the VSI's"
+
+  validation {


In order to use this validation, users need to be on terraform version >=1.9.0. So can we bump the required terraform version in the module's version.tf as all submodules and examples?

Updated to 1.9.0

ocofaigh · 2025-01-14T12:00:32Z

examples/complete/variables.tf

@@ -51,3 +51,9 @@ variable "secondary_use_vsi_security_group" {
  type        = bool
  default     = false
 }
+
+variable "enable_dedicated_host" {


no need to expose a new variable in the example - we don't want to make examples flexible solutions. Suggest to just hard code this to true in the example main.tf

Can you also update the examples/complete/README.md file to mention about dedicated host

ocofaigh · 2025-01-14T12:03:26Z

variables.tf

+variable "enable_dedicated_host" {
+  type        = bool
+  default     = false
+  description = "Setting this option to true will enable dedicated hosts for the VSI's, the default value is set to false. Refer [Understanding Dedicated Hosts](https://cloud.ibm.com/docs/vpc?topic=vpc-creating-dedicated-hosts-instances&interface=ui#about-dedicated-hosts) for more details"


update the description to mention that when set to true, a value is required for the dedicated_host_id input

ocofaigh · 2025-01-14T12:04:18Z

variables.tf

+
+  validation {
+    condition     = var.enable_dedicated_host == false || (var.enable_dedicated_host == true && var.dedicated_host_id != null)
+    error_message = "When enable_dedicated_host is set to true, provide dedicated_host_id or new dedicated host will be created."


"new dedicated host will be created"? Should this say "new dedicated host will not be created"

ocofaigh · 2025-01-14T12:05:10Z

variables.tf

+variable "dedicated_host_id" {
+  type        = string
+  default     = null
+  description = "ID of the dedicated host for hosting the VSI's"


Update the description to mention that enable_dedicated_host needs to be set to true if passing a dedicated host ID

ocofaigh · 2025-01-14T12:05:58Z

examples/complete/main.tf

+}
+
+#############################################################################
+# Dedicated Host


maybe put the code for dedicated host above the VSI code since host is needed before VSI

ocofaigh · 2025-01-14T12:06:32Z

examples/complete/main.tf

@@ -24,7 +24,7 @@ module "resource_group" {

 module "key_protect_all_inclusive" {
  source                    = "terraform-ibm-modules/kms-all-inclusive/ibm"
-  version                   = "4.19.2"
+  version                   = "4.19.1"


this is an older version to whats in main branch - suggest to change it back

ocofaigh

Left some comments. Can you also expose this feature in the fscloud submodule code (found in modules/fscloud)

Louies-Jhony · 2025-01-14T12:19:51Z

/run pipeline

Louies-Jhony · 2025-01-14T14:30:38Z

/run pipeline

Louies-Jhony · 2025-01-15T03:29:41Z

/run pipeline

Louies-Jhony · 2025-01-15T05:38:42Z

/run pipeline

Louies-Jhony · 2025-01-15T15:55:55Z

@ocofaigh

For dedicated host testing, the existing VSI module create VSI's with placement group across zones. Since VSI can be placed either in a placement group or a dedicated host, I have introduced a separate module for VSI which will create a VSI in a dedicated host on zone-1.

On the PR test go file, for complete and upgrade test cases, we are using the same setup.(i.e same folder COMPLETE)
All test functions runs in parallel which creates quota issue. Hence dedicated host cannot be included on fscloud or upgrade test cases.

I have addressed all other comments.

Please advice.

Louies-Jhony · 2025-01-15T15:56:42Z

/run pipeline

Aashiq-J · 2025-01-21T06:06:41Z

This PR no longer valid.
Changes made in #780

Test for a pipeline test

4a7677e

Louies-Jhony requested review from toddgiguere and Aashiq-J as code owners January 6, 2025 15:03

Changes for dedicated host

fe6ee98

Changes for dedicated host

719b0b4

Adding new test for Dedicated host

d585812

Changes to Dedicated host

4506d8b

Changes to Dedicated host

1557a74

Update for test validation

8902766

Louies-Jhony added 2 commits January 13, 2025 20:28

Merge branch 'terraform-ibm-modules:main' into JL_4565_main

e15b3d3

Changes for Dedicated Host

360a121

Changes to dedicated host

6fe0fb7

Update for Dedicated host

0b0f662

Update for Dedicated host

b441d7c

Update for Dedicated host

95179fb

ocofaigh reviewed Jan 14, 2025

View reviewed changes

ocofaigh requested changes Jan 14, 2025

View reviewed changes

Update for Dedicated host

89cac39

Update for Dedicated host

0bbd5cc

Update for Dedicated host

5c3cf5d

Changes for Dedicated Host

a88387b

Louies-Jhony changed the title ~~Test for a pipeline build~~ Changes for Dedicated Host with Landing Zone VSI - Reg Jan 15, 2025

Louies-Jhony mentioned this pull request Jan 16, 2025

Changes for Dedicated Host #780

Merged

4 tasks

Aashiq-J closed this Jan 21, 2025

Changes for Dedicated Host with Landing Zone VSI - Reg #777

Changes for Dedicated Host with Landing Zone VSI - Reg #777

Conversation

Louies-Jhony commented Jan 6, 2025

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

For mergers

Louies-Jhony commented Jan 6, 2025

Louies-Jhony commented Jan 7, 2025

Louies-Jhony commented Jan 7, 2025

Louies-Jhony commented Jan 7, 2025

Louies-Jhony commented Jan 9, 2025

Louies-Jhony commented Jan 9, 2025

Louies-Jhony commented Jan 9, 2025

Louies-Jhony commented Jan 13, 2025

Louies-Jhony commented Jan 14, 2025

Louies-Jhony commented Jan 14, 2025

Louies-Jhony commented Jan 14, 2025

Louies-Jhony commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocofaigh Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocofaigh left a comment

Choose a reason for hiding this comment

Louies-Jhony commented Jan 14, 2025

Louies-Jhony commented Jan 14, 2025

Louies-Jhony commented Jan 15, 2025

Louies-Jhony commented Jan 15, 2025

Louies-Jhony commented Jan 15, 2025 • edited Loading

Louies-Jhony commented Jan 15, 2025

Aashiq-J commented Jan 21, 2025

ocofaigh Jan 14, 2025 •

edited

Loading

Louies-Jhony commented Jan 15, 2025 •

edited

Loading