-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Promoting Managed Kafka Cluster and Topic resources to GA #12264
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @SarahFrench, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 5 Click here to see the affected service packages
🔴 Tests were added that are GA-only additions and require manual runs:
View the build log |
Hi! Our GA is technically November 12th. Is it possible to submit this before that date? Our beta and GA endpoints are the same. |
Review note: GA testing here
Hi @jessdejong - we can get this merged today, assuming the tests pass etc, but it's too late to be included in the release scheduled for Monday 11th Nov. The only changes that we would cherry-pick into a prepared release are bug fixes. I'm afraid this PR's changes would need to go out in the release on November 18th. |
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'm seeing a problem running the test TestAccManagedKafkaCluster_managedkafkaClusterCmekExample
with the GA provider:
------- Stdout: -------
=== RUN TestAccManagedKafkaCluster_managedkafkaClusterCmekExample
=== PAUSE TestAccManagedKafkaCluster_managedkafkaClusterCmekExample
=== CONT TestAccManagedKafkaCluster_managedkafkaClusterCmekExample
resource_managed_kafka_cluster_generated_test.go:94: Step 1/2 error: Error running pre-apply refresh: exit status 1
Error: Invalid resource type
on terraform_plugin_test.tf line 20, in resource "google_project_service_identity" "kafka_service_identity":
20: resource "google_project_service_identity" "kafka_service_identity" {
The provider hashicorp/google does not support resource type
"google_project_service_identity".
--- FAIL: TestAccManagedKafkaCluster_managedkafkaClusterCmekExample (1.68s)
FAIL
The google_project_service_identity
resource is only in the Beta provider, so that test will need to use google-beta still. I believe that to solve this you just need to re-add min_version: 'beta'
to the example named managedkafka_cluster_cmek
in mmv1/products/managedkafka/Cluster.yaml.
Hi @SarahFrench, thanks for letting me know about the releases. Given that we can not make it into the Monday release, I would like to still get this in ASAP so that we can be a part of the following release. Thanks! Also -- I'm curious where you're seeing that the CMEK test is failing. Based on this issue comment, it looks like all of the GA tests are passing. But anyway, I took your advice and added the |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 5 Click here to see the affected service packages
🔴 Tests were added that are GA-only additions and require manual runs:
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
Ok, looks like we have a new error on the CMEK test:
I'm going to try and add |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 5 Click here to see the affected service packages
🔴 Tests were added that are GA-only additions and require manual runs:
View the build log |
Hi! On our PRs we run all the automated tests with the Beta version of the provider. I believe that comment is highlighting to the reviewer that those tests have newly been added to the GA provider code but require manual testing as while they pass for the Beta provider the aren't guaranteed to also pass for the GA provider. You won't be able to access this, but I ran the tests here using the GA provider code and saw the error. I'll double check your latest changes. Edit: new GA test run here. |
I'm unfortunately not able to access the team city link. Can you let me know when/if the tests complete? @SarahFrench |
Hi @SarahFrench, were there any failures on the manual tests? |
The tests all passed, and the TestAccManagedKafkaCluster_managedkafkaClusterCmekExample test is still Beta-only as needed |
https://b.corp.google.com/issues/371049718
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.