Skip to content

Conversation

@iamar7
Copy link
Member

@iamar7 iamar7 commented Nov 11, 2025

Description

Resolves: #102

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.

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

Have you run the script in an account where no primary metadata region has been set? What happens?

The script is currently hard coded to production endpoints, meaning it wont work in stage. The script needs to be able to handle environment variable overrides that someone might make. Same way we did in confirm_lb_active.sh for example.

@iamar7
Copy link
Member Author

iamar7 commented Nov 12, 2025

Have you run the script in an account where no primary metadata region has been set? What happens?

The script is currently hard coded to production endpoints, meaning it wont work in stage. The script needs to be able to handle environment variable overrides that someone might make. Same way we did in confirm_lb_active.sh for example.

In an account where the primary_metadata_region is not set the script returns an empty string.

Screenshot 2025-11-12 at 10 25 12 PM

@iamar7 iamar7 marked this pull request as ready for review November 17, 2025 09:01
@iamar7 iamar7 requested a review from Khuzaima05 as a code owner November 17, 2025 09:01
@iamar7
Copy link
Member Author

iamar7 commented Nov 17, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 17, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 17, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 18, 2025

This is an expected failure since we were updating the metrics_routing_settings by default. Skipping Upgrade Test

2025/11/17 23:52:14 Terraform plan | Terraform will perform the following actions:
         2025/11/17 23:52:14 Terraform plan | 
         2025/11/17 23:52:14 Terraform plan |   # module.metrics_routing[0].ibm_metrics_router_settings.metrics_router_settings[0] will be destroyed
         2025/11/17 23:52:14 Terraform plan |   # (because index [0] is out of range for count)
         2025/11/17 23:52:14 Terraform plan |   - resource "ibm_metrics_router_settings" "metrics_router_settings" {
         2025/11/17 23:52:14 Terraform plan |       - id                        = "br-sao" -> null
         2025/11/17 23:52:14 Terraform plan |       - permitted_target_regions  = [] -> null
         2025/11/17 23:52:14 Terraform plan |       - primary_metadata_region   = "br-sao" -> null
         2025/11/17 23:52:14 Terraform plan |       - private_api_endpoint_only = false -> null
         2025/11/17 23:52:14 Terraform plan |         # (1 unchanged attribute hidden)
         2025/11/17 23:52:14 Terraform plan |     }
         2025/11/17 23:52:14 Terraform plan | 
Screenshot 2025-11-18 at 6 22 52 AM

@iamar7
Copy link
Member Author

iamar7 commented Nov 18, 2025

/run pipeline

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

@iamar7 Can you show in todays deep dive for team feedback please?

@iamar7
Copy link
Member Author

iamar7 commented Nov 24, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 24, 2025

Running the fully configurable DA on an account which already has primary_metadata_region set resulted in showing that 6 resource will be created:

Screenshot 2025-11-24 at 4 23 33 PM

@iamar7
Copy link
Member Author

iamar7 commented Nov 24, 2025

Running the fully configurable DA on a new account which doesn't have primary_metadata_region set resulted in showing that 7 resource will be created including the account_settings block to be added:

Screenshot 2025-11-24 at 4 32 26 PM Screenshot 2025-11-24 at 4 31 06 PM

@iamar7
Copy link
Member Author

iamar7 commented Nov 24, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 26, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 27, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Nov 27, 2025

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Dec 1, 2025

/run pipeline

import sys
import time

import requests
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot use requests - this is an external pip package that requires installation. I did mention this on the deep dive. We can use the python built in http library: https://docs.python.org/3/library/http.client.html

Check out our internal sdnlb module which recently was updated to use this library too.

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

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.

Investigate requested change around the automatic setting of the metrics primary metadata region

3 participants