Skip to content

HIVE-2382: Add AWS user-provisioned DNS OTE coverage - #2933

Open
bandrade wants to merge 2 commits into
openshift:masterfrom
bandrade:HIVE-2382-user-provisioned-dns-ote
Open

HIVE-2382: Add AWS user-provisioned DNS OTE coverage#2933
bandrade wants to merge 2 commits into
openshift:masterfrom
bandrade:HIVE-2382-user-provisioned-dns-ote

Conversation

@bandrade

@bandrade bandrade commented Jul 29, 2026

Copy link
Copy Markdown

Summary

  • port the OCP-83733 AWS ClusterDeployment and ClusterPool user-provisioned DNS scenarios into the Hive OTE suite
  • verify Hive preserves userProvisionedDNS: Enabled and does not create a managed DNSZone
  • publish and clean up test-owned API and ingress CNAME records
  • verify both installed clusters are reachable with their generated kubeconfigs

Validation

  • go test -vet=off ./hive -run "^$" from test/ote
  • make -C test/ote build
  • make -C test/ote -f bindata.mk verify-testdata
  • git diff --check

The default go test vet pass still reports existing non-constant-format findings elsewhere in hive_aws.go and hive_util.go; the OTE build used by CI succeeds.

Summary by CodeRabbit

  • New Features

    • Added support for user-provisioned DNS in AWS cluster and cluster pool configurations.
    • Extended install-config templating to include userProvisionedDNS (defaulting to Disabled) and optional template reference injection.
  • Tests

    • Added end-to-end coverage validating AWS clusters and cluster pools with user-provisioned DNS: no managed DNS zone creation, DNS records publication, and successful cluster reachability via kubeconfig.
  • Bug Fixes

    • Improved custom AWS DNS publishing and cleanup by waiting for Route 53 CNAME propagation.

Port the ClusterDeployment and ClusterPool scenarios from OCP-83733 into the Hive OTE suite. The tests publish and clean up their own API and ingress DNS records, verify Hive does not create a managed DNSZone, and prove the installed clusters are reachable.

Assisted-by: GPT-5
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 29, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 29, 2026

Copy link
Copy Markdown

@bandrade: This pull request references HIVE-2382 which is a valid jira issue.

Details

In response to this:

Summary

  • port the OCP-83733 AWS ClusterDeployment and ClusterPool user-provisioned DNS scenarios into the Hive OTE suite
  • verify Hive preserves userProvisionedDNS: Enabled and does not create a managed DNSZone
  • publish and clean up test-owned API and ingress CNAME records
  • verify both installed clusters are reachable with their generated kubeconfigs

Validation

  • go test -vet=off ./hive -run "^$" from test/ote
  • make -C test/ote build
  • make -C test/ote -f bindata.mk verify-testdata
  • git diff --check

The default go test vet pass still reports existing non-constant-format findings elsewhere in hive_aws.go and hive_util.go; the OTE build used by CI succeeds.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci
openshift-ci Bot requested review from dlom and jstuever July 29, 2026 15:45
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d1ca29d-c349-481e-9c6e-0a5d331f9a6a

📥 Commits

Reviewing files that changed from the base of the PR and between e9684ec and 17247f9.

📒 Files selected for processing (1)
  • test/ote/hive/hive_aws_custom_dns.go

📝 Walkthrough

Walkthrough

AWS Hive test coverage now supports user-provisioned DNS for ClusterDeployments and ClusterPools. Templates pass the setting, the custom DNS manager waits for Route 53 propagation, and tests verify cluster installation and access.

Changes

AWS user-provisioned DNS

Layer / File(s) Summary
Template configuration plumbing
test/ote/hive/hive_util.go, test/ote/hive/testdata/aws-install-config.yaml, test/ote/hive/testdata/clusterpool-custom-dns.yaml
Install-config and ClusterPool helpers pass user-provisioned DNS and install-config secret template parameters into rendered resources.
AWS custom DNS propagation
test/ote/hive/hive_aws_custom_dns.go, test/ote/go.mod
The DNS manager discovers AWS load balancers, publishes API and ingress CNAMEs, waits for Route 53 propagation during updates and cleanup, and uses the updated AWS SDK modules.
ClusterDeployment and ClusterPool validation
test/ote/hive/hive_aws.go
Adds AWS tests that validate rendered DNS settings, absence of dnszone, custom DNS publication, installation completion, and kubeconfig-based cluster access.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HiveTest
  participant Hive
  participant AWSCustomDNSManager
  participant AWS
  participant Route53
  participant Cluster
  HiveTest->>Hive: Create user-provisioned DNS ClusterDeployment or ClusterPool
  Hive-->>HiveTest: Render install-config and omit dnszone
  HiveTest->>AWSCustomDNSManager: Publish API and ingress records
  AWSCustomDNSManager->>AWS: Find tagged load balancers
  AWS-->>AWSCustomDNSManager: Return load balancer DNS names
  AWSCustomDNSManager->>Route53: Upsert CNAME records
  Route53-->>AWSCustomDNSManager: Confirm propagated changes
  HiveTest->>Cluster: Retrieve kubeconfig and run oc get co
  Cluster-->>HiveTest: Return cluster operator status
Loading

Suggested reviewers: dlom, jstuever

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding AWS user-provisioned DNS OTE coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/ote/hive/hive_aws_custom_dns.go`:
- Around line 127-150: Update awsCustomDNSManager.upsertCNAME to retain the
ChangeResourceRecordSets change ID and wait until Route 53 reports the change as
INSYNC before recording the resource and returning true. Apply the same
synchronization to the corresponding delete path, waiting for each change ID (or
using the available Route 53 waiter) before cleanup proceeds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b22612b3-8c56-46d6-850d-1947133f0ed2

📥 Commits

Reviewing files that changed from the base of the PR and between c04833d and e9684ec.

⛔ Files ignored due to path filters (1)
  • test/ote/go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • test/ote/go.mod
  • test/ote/hive/hive_aws.go
  • test/ote/hive/hive_aws_custom_dns.go
  • test/ote/hive/hive_util.go
  • test/ote/hive/testdata/aws-install-config.yaml
  • test/ote/hive/testdata/clusterpool-custom-dns.yaml

Comment thread test/ote/hive/hive_aws_custom_dns.go
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.51%. Comparing base (c04833d) to head (17247f9).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2933   +/-   ##
=======================================
  Coverage   50.51%   50.51%           
=======================================
  Files         282      282           
  Lines       34485    34485           
=======================================
  Hits        17420    17420           
  Misses      15707    15707           
  Partials     1358     1358           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@2uasimojo

Copy link
Copy Markdown
Member

/approve

Your change did not introduce new vulns; the existing ones will be dealt with elsewhere.

/override ci/prow/security

(I'll need to repeat this ^ if we iterate.)

Where/how are you validating this? It's not covered by any of the prow presubmits (and I don't know how to trigger a periodic with a PR included).

@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, bandrade

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2026
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security

Details

In response to this:

/approve

Your change did not introduce new vulns; the existing ones will be dealt with elsewhere.

/override ci/prow/security

(I'll need to repeat this ^ if we iterate.)

Where/how are you validating this? It's not covered by any of the prow presubmits (and I don't know how to trigger a periodic with a PR included).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@bandrade: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Wait for Route 53 changes to reach INSYNC before tests continue or cleanup completes.

Assisted-by: GPT-5
@bandrade

bandrade commented Jul 30, 2026

Copy link
Copy Markdown
Author

Thanks. The validation path is:

  • local OTE compile: go test -vet=off ./hive -run "^$"
  • extension build and discovery: make -C test/ote build; both OCP-83733 cases appear in openshift-tests-extension list
  • embedded testdata verification: make -C test/ote -f bindata.mk verify-testdata
  • post-merge execution: periodic-ci-openshift-hive-master-periodic-e2e-ote-sd-rosa, configured in openshift-hive-master__periodic.yaml, builds the Hive extension from master, targets OCP 4.22, and selects TEST_TAG=HiveSDRosa

The periodic lane is active (latest run on July 25 passed), but it does not have a presubmit variant that injects the PR branch, so the first full AWS execution of these cases is the post-merge weekly run.

I also addressed the Route 53 propagation review finding in 17247f9d0: create and delete changes now wait for INSYNC. The OTE compile, extension build, testdata verification, and git diff --check pass locally.

This PR intentionally establishes the two public AWS cases that previously passed OCP-83733. PrivateLink remains an open HIVE-2382 validation scenario; I am coordinating the expected custom-DNS/reachability flow before extending automation for that topology.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants