Skip to content

feat: fully configurable app config da #212

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

mukulpalit-ibm
Copy link

@mukulpalit-ibm mukulpalit-ibm commented Apr 28, 2025

Description

  • This PR creates a new Fully Configurable DA for App Configurations along with all the necessary files.
  • Currently this PR will skip upgrade tests as the DA is not available in main.
    Issue

Sample Catalog for DA
Screenshot 2025-04-28 at 3 03 22 PM
Screenshot 2025-04-28 at 3 03 42 PM

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.

@mukulpalit-ibm mukulpalit-ibm changed the title 13296 fully configurable da feat: fully configurable app config da Apr 28, 2025
@mukulpalit-ibm
Copy link
Author

/run pipeline

Comment on lines 1 to 5
{
"ibmcloud_api_key": $VALIDATION_APIKEY,
"app_config_tags": $TAGS,
"existing_resource_group_name": "geretain-test-app-config"
}
Copy link
Member

Choose a reason for hiding this comment

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

This file should have all required inputs to run your code. I think you need region and service name here.
"existing_resource_group_name", "geretain-test-app-config" may not be existing one, so you should use "geretain-test-resources"
Prefix also should be added.

Copy link
Author

@mukulpalit-ibm mukulpalit-ibm May 2, 2025

Choose a reason for hiding this comment

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

added prefix and region . I think service is not needed here ?

@maheshwarishikha
Copy link
Member

  • Architecture diagram should say official name of the service - App Configuration
  • Why are we not running tests in parallel?

@maheshwarishikha
Copy link
Member

maheshwarishikha commented Apr 30, 2025

renovate.json needs to be updated. Refer [this](https://github.com/terraform-ibm-modules/terraform-ibm-observability-da/blob/main/renovate.json). -- NOT REQUIRED. Please ignore.

@maheshwarishikha
Copy link
Member

Regarding tests (DA test and upgradeDA test):

  • you are running existing resources to create resource group only. Instead you can use geretain-test-resources resource group.
  • As the module/DA does not support existing App Config instances currently, we can directly use an existing resource group and run test in Schematics. This will be simpler and more consistent with our current approach.

@mukulpalit-ibm
Copy link
Author

mukulpalit-ibm commented May 2, 2025

  • Architecture diagram should say official name of the service - App Configuration
  • Why are we not running tests in parallel?

@maheshwarishikha
Regarding tests not running in parallel : The tests were failing as for testing we are creating the free plan for app config which is allowed only 1 per account. Therefore there were two options either to upgrade the plan for testing to basic or standard or the run tests one by one. I want to know what is the best approach for such cases

@ocofaigh
Copy link
Member

ocofaigh commented May 2, 2025

@mukulpalit-ibm just change the plan used in the example and run tests in parallel

@mukulpalit-ibm
Copy link
Author

/run pipeline

Copy link
Member

@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.

@mukulpalit-ibm #214 has just been merged. Can you also expose this new feature in the DA please?

mark_ready: true
install_type: fullstack
scc:
instance_id:
Copy link
Member

Choose a reason for hiding this comment

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

add this 1c7d5f78-9262-44c3-b779-b28fe4d88c37 as instance_id

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.

3 participants