NE-1277: status: Add Gateway API objects to relatedObjects #933

Miciah · 2023-05-18T23:42:51Z

pkg/operator/controller/names.go (GlobalOperatorsNamespace): New const for the "openshift-operators" namespace.
(ServiceMeshSubscriptionName): Use GlobalOperatorsNamespace.
pkg/operator/controller/status/controller.go (New): Add a watch for subscriptions so that the status controller updates relatedObjects when the OSSM subscription is added or removed.
(Config): Add GatewayAPIEnabled field.
(Reconcile): If GatewayAPIEnabled is true, add resources related to Gateway API to relatedObjects. If haveOSSMSubscription is true, add resources that require the subscription.
(operatorState): Add haveOSSMSubscription field.
(getOperatorState): Set haveOSSMSubscription.
pkg/operator/operator.go (New): Watch the "openshift-operators" namespace, and specify GatewayAPIEnabled in the status controller's config.

openshift-ci-robot · 2023-05-18T23:42:54Z

@Miciah: This pull request references NE-1277 which is a valid jira issue.

In response to this:

pkg/operator/controller/names.go (GlobalOperatorsNamespace): New const for the "openshift-operators" namespace.
(ServiceMeshSubscriptionName): Use GlobalOperatorsNamespace.

pkg/operator/controller/status/controller.go (New): Add a watch for subscriptions so that the status controller updates relatedObjects when the OSSM subscription is added or removed.
(Config): Add GatewayAPIEnabled field.
(Reconcile): If GatewayAPIEnabled is true, add resources related to Gateway API to relatedObjects. If haveOSSMSubscription is true, add resources that require the subscription.
(operatorState): Add haveOSSMSubscription field.
(getOperatorState): Set haveOSSMSubscription.

pkg/operator/operator.go (New): Watch the "openshift-operators" namespace, and specify GatewayAPIEnabled in the status controller's config.

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/test-infra repository.

openshift-ci · 2023-05-18T23:42:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-bot · 2023-08-29T09:00:17Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2023-09-29T00:30:33Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-ci-robot · 2023-09-29T00:30:37Z

@Miciah: This pull request references NE-1277 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

pkg/operator/controller/names.go (GlobalOperatorsNamespace): New const for the "openshift-operators" namespace.
(ServiceMeshSubscriptionName): Use GlobalOperatorsNamespace.

pkg/operator/controller/status/controller.go (New): Add a watch for subscriptions so that the status controller updates relatedObjects when the OSSM subscription is added or removed.
(Config): Add GatewayAPIEnabled field.
(Reconcile): If GatewayAPIEnabled is true, add resources related to Gateway API to relatedObjects. If haveOSSMSubscription is true, add resources that require the subscription.
(operatorState): Add haveOSSMSubscription field.
(getOperatorState): Set haveOSSMSubscription.

pkg/operator/operator.go (New): Watch the "openshift-operators" namespace, and specify GatewayAPIEnabled in the status controller's config.

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/test-infra repository.

openshift-bot · 2023-10-29T08:00:30Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2023-10-29T08:01:01Z

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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/test-infra repository.

Miciah · 2025-03-06T20:38:36Z

/reopen

openshift-ci-robot · 2025-03-06T20:41:42Z

@Miciah: This pull request references NE-1277 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

pkg/operator/controller/names.go (GlobalOperatorsNamespace): New const for the "openshift-operators" namespace.
(ServiceMeshSubscriptionName): Use GlobalOperatorsNamespace.

pkg/operator/controller/status/controller.go (New): Add a watch for subscriptions so that the status controller updates relatedObjects when the OSSM subscription is added or removed.
(Config): Add GatewayAPIEnabled field.
(Reconcile): If GatewayAPIEnabled is true, add resources related to Gateway API to relatedObjects. If haveOSSMSubscription is true, add resources that require the subscription.
(operatorState): Add haveOSSMSubscription field.
(getOperatorState): Set haveOSSMSubscription.

pkg/operator/operator.go (New): Watch the "openshift-operators" namespace, and specify GatewayAPIEnabled in the status controller's config.

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 · 2025-03-06T20:42:00Z

@Miciah: Reopened this PR.

In response to this:

/reopen

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.

Miciah · 2025-03-06T21:06:25Z

https://github.com/openshift/cluster-ingress-operator/compare/6246acc7f400ebe0376589efa793d8033c5a8683..78c31dd241a7d0e8a579a4d1f8b870d569c1c96d rebases onto #1152 in order to get the Gateway API v1 and Sail Operator v1 CRDs. Some other changes were made redundant by #1115 and are thus dropped from this PR.

Miciah · 2025-03-06T21:06:46Z

/test e2e-aws-operator

Miciah · 2025-03-11T17:07:04Z

Rebased onto #1152.

candita · 2025-03-20T19:23:56Z

/assign

candita · 2025-03-22T02:38:35Z

pkg/operator/controller/status/controller.go

+				Namespace: subscriptionName.Namespace,
+				Name:      subscriptionName.Name,
+			})
+			if state.IngressNamespace != nil {


Is it possible for a gateway to be installed outside the IngressNamespace and still be a related object?

Also, how about HTTPRoutes, GRPCRoutes, and ReferenceGrants?

Is there a way to make sure the pod that runs conformance tests ends up as a related object so we can check the logs in artifacts?

Is it possible for a gateway to be installed outside the IngressNamespace and still be a related object?

I don't think it would be appropriate to include objects that constitute user workload, and for component-owned objects, the precedent is that the component that owns the object specifies it in its relatedObjects field. For example, the authentication operator specifies its route in the authentication clusteroperator's relatedObjects field: https://github.com/openshift/cluster-authentication-operator/blob/11b2201203fd5ba0dec937015fa542f4cd4c8879/pkg/operator/starter.go#L566

Also, how about HTTPRoutes, GRPCRoutes, and ReferenceGrants?

We don't have any. Again, we wouldn't want to include other components' objects or objects related to user workload. This is analogous to how the ingress clusteroperator specifies ingresscontrollers but not routes in relatedObjects. (Come to think of it, maybe we ought to specify the canary route. I can file an issue for that if you agree.)

Is there a way to make sure the pod that runs conformance tests ends up as a related object so we can check the logs in artifacts?

I think that that would be misuse of relatedObjects. Aside from that, the must-gather artifacts can only include objects that exist when must-gather runs, and CI runs must-gather after the tests have finished, so unless the conformance tests leak objects, must-gather wouldn't collect them anyway.

lihongan · 2025-03-26T01:31:40Z

/test e2e-aws-gatewayapi-conformance

Miciah · 2025-03-26T14:30:20Z

https://github.com/openshift/cluster-ingress-operator/compare/bef05ce144a269f1a170e1e8c22e53a769a5128b..86f0c17f610eee033337b3710a0e728a02319db6 adds "dnsrecords" in the "openshift-ingress" namespace to the expected relatedObjects in TestClusterOperatorStatusRelatedObjects and adds some logging to help diagnose the TestCanaryRoute failure on e2e-aws-operator.

Miciah · 2025-03-26T20:28:21Z

https://github.com/openshift/cluster-ingress-operator/compare/86f0c17f610eee033337b3710a0e728a02319db6..77beec1a8b6af8c8c5ce00f29f7a60683a84cd75 adds some more logging to help diagnose test failures.

candita · 2025-03-27T16:14:41Z

/test e2e-aws-operator-techpreview

candita · 2025-03-27T17:25:48Z

test/e2e/canary_test.go

@@ -36,65 +36,66 @@ import (
 func TestCanaryRoute(t *testing.T) {
 	kubeConfig, err := config.GetConfig()
 	if err != nil {
-		t.Fatalf("failed to get kube config: %v", err)
+		t.Fatalf("Failed to get kube config: %v", err)


If any more test changes are needed, could you please put them in a separate PR?

Do you mean additional changes to TestCanaryRoute or to other tests?

I can split the other test fixes off into separate PRs and Jira issues if you prefer. I've just been adding them as commits in this PR as I diagnose CI failures that impact this PR.

This commit resolves NE-1277. https://issues.redhat.com/browse/NE-1277 * pkg/operator/controller/status/controller.go (New): Add a watch for subscriptions so that the status controller updates relatedObjects with the OSSM subscription when it is added or removed. (Config): Add GatewayAPIEnabled field. (Reconcile): Add dnsrecords in the operand namespace to relatedObjects. If GatewayAPIEnabled is true, add resources related to Gateway API. If haveOSSMSubscription is true, add resources that require the subscription. (operatorState): Add haveOSSMSubscription field. (getOperatorState): Set haveOSSMSubscription. * pkg/operator/operator.go (New): Specify GatewayAPIEnabled in the status controller's config. * test/e2e/operator_test.go (TestClusterOperatorStatusRelatedObjects): Expect to observe "dnsrecords" for the "openshift-ingress" namespace as well as "gatewayclasses" in relatedObjects.

* test/e2e/canary_test.go (TestCanaryRoute): Add some more logging to make it easier to diagnose test failures.

candita · 2025-03-27T20:33:36Z

test/e2e/operator_test.go

+		{
+			Group:     iov1.GroupVersion.Group,
+			Resource:  "dnsrecords",
+			Namespace: "openshift-ingress",
+		},


See https://github.com/openshift/cluster-ingress-operator/pull/933/files#r2017559921

candita · 2025-03-27T20:40:59Z

test/e2e/operator_test.go

+		// This test runs before TestGatewayAPI, so we do *not* expect
+		// to see subscriptions, istios, or gateways in relatedObjects.


If this runs before TestGatewayAPI, what created the gatewayClass?

The gatewayapi controller creates the CRD if the featuregate is enabled. We only need the CRD to exist in order to add the gatewayclasses resource to relatedObjects; we don't need any CR to exist. (However, if TestGatewayAPI does run as part of CI, then having "gatewayclasses" in relatedObjects will cause the CI job to gather the gatewayclass that TestGatewayAPI created when the CI job runs the must-gather step, after the test step.)

candita · 2025-03-27T20:42:34Z

test/e2e/util_gatewayapi_test.go

@@ -562,14 +563,14 @@ func assertGatewaySuccessful(t *testing.T, namespace, name string) (*gatewayapiv

 	err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) {
 		if err := kclient.Get(context, nsName, gw); err != nil {
-			t.Logf("failed to get gateway %s, retrying...", name)
+			t.Logf("Failed to get gateway %v: %v", nsName, err)


nit:

Suggested change

t.Logf("Failed to get gateway %v: %v", nsName, err)

t.Logf("Failed to get gateway %v: %v, retrying...", nsName, err)

Done in https://github.com/openshift/cluster-ingress-operator/compare/77beec1a8b6af8c8c5ce00f29f7a60683a84cd75..92e89a2a02fccb3d149e3e0491a30f19fe160774.

candita · 2025-03-27T20:44:01Z

test/e2e/util_gatewayapi_test.go

@@ -598,15 +601,15 @@ func assertHttpRouteSuccessful(t *testing.T, namespace, name string, gateway *ga
 	// Wait 1 minute for parent/s to update
 	err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) {
 		if err := kclient.Get(context, nsName, httproute); err != nil {
-			t.Logf("failed to get httproute %s/%s, retrying...", namespace, name)
+			t.Logf("Failed to get httproute %v: %v", nsName, err)


A couple more cases here that had the "retrying" before. Also, I expect that the error message is not going to change much on each iteration.

Added back "retrying" in https://github.com/openshift/cluster-ingress-operator/compare/77beec1a8b6af8c8c5ce00f29f7a60683a84cd75..92e89a2a02fccb3d149e3e0491a30f19fe160774.

I don't think it's worth the effort to save and compare the last err.String() to make logging slightly more compact. If it is worth that effort, then we should probably write a getWithRetry helper and update all tests to use it.

candita · 2025-03-27T20:47:01Z

test/e2e/util_gatewayapi_test.go

 	if err := assertDNSRecord(t, dnsRecordName); err != nil {
 		return err
 	}

-	// Wait and check that the dns name resolves first. Takes a long time, so
-	// if the hostname is actually an IP address, skip this.
+	t.Logf("Attempting to resolve %s...", hostname)


To be precise, it won't attempt to resolve the hostname if it is actually an IP address. This log message is in accurate in that case.

I put the comment back in https://github.com/openshift/cluster-ingress-operator/compare/77beec1a8b6af8c8c5ce00f29f7a60683a84cd75..92e89a2a02fccb3d149e3e0491a30f19fe160774.

candita · 2025-03-27T20:49:05Z

test/e2e/util_gatewayapi_test.go

@@ -843,20 +846,21 @@ func assertDNSRecord(t *testing.T, recordName types.NamespacedName) error {

 	err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) {
 		if err := kclient.Get(context, recordName, dnsRecord); err != nil {
-			t.Logf("failed to get DNSRecord %s/%s: %v, retrying...", recordName.Namespace, recordName.Name, err)
+			t.Logf("Failed to get DNSRecord %v: %v", recordName, err)


Again a case where "retrying" in the log message is helpful.

Added back "retrying" in https://github.com/openshift/cluster-ingress-operator/compare/77beec1a8b6af8c8c5ce00f29f7a60683a84cd75..92e89a2a02fccb3d149e3e0491a30f19fe160774.

Add more logging to functions that testGatewayAPIObjects uses in order to make it easier to diagnose test failures. * test/e2e/gateway_api_test.go (ensureGatewayObjectSuccess): Remove t.Helper() and add more logging. This function has core test logic that can mark the test as failed, and marking this function as a helper obfuscates the point of failure. * test/e2e/util_gatewayapi_test.go (assertGatewayClassSuccessful): Add more logging. (assertGatewaySuccessful): Add more logging. (assertHttpRouteSuccessful): Add more logging. (assertHttpRouteConnection): Remove t.Helper() and add more logging. (assertDNSRecord): Add more logging.

Miciah · 2025-03-27T23:04:11Z

https://github.com/openshift/cluster-ingress-operator/compare/77beec1a8b6af8c8c5ce00f29f7a60683a84cd75..92e89a2a02fccb3d149e3e0491a30f19fe160774 moves an import and makes some log messages more consistent.

Miciah · 2025-03-28T00:02:37Z

Some earlier runs of e2e-aws-operator and e2e-aws-operator-techpreview failed on TestIngressControllerCustomEndpoints. This test is failing on multiple PRs, and these failures are being tracked with OCPBUGS-53432.

Miciah · 2025-03-28T00:03:15Z

Many CI errors earlier today were caused by a Marketplace outage.

candita · 2025-03-28T19:20:07Z

/lgtm
/approve

openshift-ci · 2025-03-28T19:20:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [candita]

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

openshift-ci-robot · 2025-03-28T19:47:20Z

/retest-required

Remaining retests: 0 against base HEAD 7f0fd6d and 2 for PR HEAD 92e89a2 in total

openshift-ci-robot · 2025-03-28T19:47:33Z

/retest-required

Remaining retests: 0 against base HEAD 7f0fd6d and 2 for PR HEAD 92e89a2 in total

openshift-ci · 2025-03-29T02:09:05Z

@Miciah: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-single-node	`92e89a2`	link	false	`/test e2e-aws-ovn-single-node`
ci/prow/okd-scos-e2e-aws-ovn	`92e89a2`	link	false	`/test okd-scos-e2e-aws-ovn`

Full PR test history. Your PR dashboard.

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.

Miciah · 2025-03-30T14:10:13Z

hypershift-e2e-aks failed because EnsureNoCrashingPods failed:

{Failed  === RUN   TestNodePool/HostedCluster2/ValidateHostedCluster/EnsureNoCrashingPods
    util.go:155: Successfully waited for kubeconfig to be published for HostedCluster e2e-clusters-nnpcz/node-pool-b94t4 in 25ms
    util.go:172: Successfully waited for kubeconfig secret to have data in 25ms
    util.go:606: Container snapshot-controller in pod csi-snapshot-controller-69dcc9f5c4-wchld has a restartCount > 0 (1)
            --- FAIL: TestNodePool/HostedCluster2/ValidateHostedCluster/EnsureNoCrashingPods (0.33s)
}

/test hypershift-e2e-aks

openshift-bot · 2025-03-31T16:30:59Z

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-ingress-operator
This PR has been included in build ose-cluster-ingress-operator-container-v4.19.0-202503311540.p0.g81e314f.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci-robot added the jira/valid-reference label May 18, 2023

openshift-ci bot added the do-not-merge/work-in-progress label May 18, 2023

openshift-merge-robot added the needs-rebase label May 31, 2023

openshift-ci bot added the lifecycle/stale label Aug 29, 2023

openshift-ci bot removed the lifecycle/stale label Sep 29, 2023

openshift-ci bot added the lifecycle/rotten label Sep 29, 2023

openshift-ci bot closed this Oct 29, 2023

openshift-ci bot reopened this Mar 6, 2025

Miciah force-pushed the NE-1277-status-add-Gateway-API-objects-to-relatedObjects branch from 6246acc to 78c31dd Compare March 6, 2025 21:04

openshift-merge-robot removed the needs-rebase label Mar 6, 2025

Miciah force-pushed the NE-1277-status-add-Gateway-API-objects-to-relatedObjects branch from 78c31dd to 459761a Compare March 11, 2025 17:06

Miciah added priority/important-soon and removed lifecycle/rotten labels Mar 12, 2025

openshift-merge-robot added the needs-rebase label Mar 20, 2025

openshift-ci bot assigned candita Mar 20, 2025

candita reviewed Mar 22, 2025

View reviewed changes

Miciah force-pushed the NE-1277-status-add-Gateway-API-objects-to-relatedObjects branch from bef05ce to 86f0c17 Compare March 26, 2025 14:29

Miciah force-pushed the NE-1277-status-add-Gateway-API-objects-to-relatedObjects branch from 86f0c17 to 77beec1 Compare March 26, 2025 20:27

candita reviewed Mar 27, 2025

View reviewed changes

Miciah added 2 commits March 27, 2025 14:03

TestCanaryRoute: Add more logging.

ff903c7

* test/e2e/canary_test.go (TestCanaryRoute): Add some more logging to make it easier to diagnose test failures.

candita reviewed Mar 27, 2025

View reviewed changes

Miciah force-pushed the NE-1277-status-add-Gateway-API-objects-to-relatedObjects branch from 77beec1 to 92e89a2 Compare March 27, 2025 23:03

openshift-ci bot added the lgtm label Mar 28, 2025

openshift-ci bot added the approved label Mar 28, 2025

openshift-merge-bot bot merged commit 81e314f into openshift:master Mar 30, 2025
17 of 19 checks passed

		// This test runs before TestGatewayAPI, so we do not expect
		// to see subscriptions, istios, or gateways in relatedObjects.

	t.Logf("Failed to get gateway %v: %v", nsName, err)
	t.Logf("Failed to get gateway %v: %v, retrying...", nsName, err)

NE-1277: status: Add Gateway API objects to relatedObjects #933

NE-1277: status: Add Gateway API objects to relatedObjects #933

Conversation

Miciah commented May 18, 2023

openshift-ci-robot commented May 18, 2023 • edited by openshift-ci bot Loading

openshift-ci bot commented May 18, 2023

openshift-bot commented Aug 29, 2023

openshift-bot commented Sep 29, 2023

openshift-ci-robot commented Sep 29, 2023 • edited by openshift-ci bot Loading

openshift-bot commented Oct 29, 2023

openshift-ci bot commented Oct 29, 2023

Miciah commented Mar 6, 2025

openshift-ci-robot commented Mar 6, 2025 • edited by openshift-ci bot Loading

openshift-ci bot commented Mar 6, 2025

Miciah commented Mar 6, 2025

Miciah commented Mar 6, 2025

Miciah commented Mar 11, 2025

candita commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lihongan commented Mar 26, 2025

Miciah commented Mar 26, 2025

Miciah commented Mar 26, 2025

candita commented Mar 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Miciah Mar 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Miciah Mar 27, 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

Choose a reason for hiding this comment

Miciah commented Mar 27, 2025

Miciah commented Mar 28, 2025

Miciah commented Mar 28, 2025 • edited Loading

candita commented Mar 28, 2025

openshift-ci bot commented Mar 28, 2025

openshift-ci-robot commented Mar 28, 2025

openshift-ci-robot commented Mar 28, 2025

openshift-ci bot commented Mar 29, 2025 • edited Loading

Miciah commented Mar 30, 2025

openshift-bot commented Mar 31, 2025

openshift-ci-robot commented May 18, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Sep 29, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 6, 2025 •

edited by openshift-ci bot

Loading

Miciah Mar 27, 2025 •

edited

Loading

Miciah Mar 27, 2025 •

edited

Loading

Miciah commented Mar 28, 2025 •

edited

Loading

openshift-ci bot commented Mar 29, 2025 •

edited

Loading