Skip to content
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

🤖 Triggering CI on branch 'release-next' after syncing to upstream/main #892

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

serverless-qe
Copy link
Collaborator

No description provided.

Copy link

openshift-ci bot commented Sep 24, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: serverless-qe
Once this PR has been reviewed and has the lgtm label, please assign retocode for approval. For more information see the Kubernetes Code Review Process.

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

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

@skonto
Copy link

skonto commented Sep 24, 2024

	0s
{Failed  panic: test timed out after 10m0s
running tests:
	TestGRPCUnaryPing (10m0s)

@skonto
Copy link

skonto commented Sep 24, 2024

/test 416-test-e2e-aws-416

@skonto
Copy link

skonto commented Sep 24, 2024

@ReToCode do we patch anything related to that test? 🤔

@skonto
Copy link

skonto commented Sep 24, 2024

It seems it fails in periodics too.

@ReToCode
Copy link
Member

Not that I'm aware of. No idea 🤷

@skonto
Copy link

skonto commented Sep 24, 2024

It seems quite deterministic.

@skonto
Copy link

skonto commented Sep 24, 2024

It fails when it goes through OCP route:

04:47:02.150 SUCCESS: 🌟 Tests have passed 🌟
gRPC test via OCP
ingress.config.openshift.io/cluster annotated
knativeserving.operator.knative.dev/knative-serving annotated
knativeserving.operator.knative.dev/knative-serving patched

@skonto skonto mentioned this pull request Sep 24, 2024
@skonto
Copy link

skonto commented Sep 24, 2024

/test 416-test-e2e-aws-416

@skonto
Copy link

skonto commented Sep 24, 2024

/test ?

Copy link

openshift-ci bot commented Sep 24, 2024

@skonto: The following commands are available to trigger required jobs:

  • /test 413-images
  • /test 413-test-e2e-aws-413
  • /test 416-images
  • /test 416-test-e2e-aws-416
  • /test 417-images
  • /test 417-test-e2e-aws-417

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-knative-serving-release-next-416-images
  • pull-ci-openshift-knative-serving-release-next-416-test-e2e-aws-416

In response to this:

/test ?

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.

@skonto
Copy link

skonto commented Sep 24, 2024

/test 413-test-e2e-aws-413

@skonto
Copy link

skonto commented Sep 24, 2024

/test 417-test-e2e-aws-417

@skonto
Copy link

skonto commented Sep 24, 2024

/override ci/prow/417-test-e2e-aws-417

Copy link

openshift-ci bot commented Sep 24, 2024

@skonto: Overrode contexts on behalf of skonto: ci/prow/417-test-e2e-aws-417

In response to this:

/override ci/prow/417-test-e2e-aws-417

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.

@skonto
Copy link

skonto commented Sep 24, 2024

/override ?

Copy link

openshift-ci bot commented Sep 24, 2024

@skonto: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • ?

Only the following failed contexts/checkruns were expected:

  • ci/prow/413-test-e2e-aws-413
  • ci/prow/416-images
  • ci/prow/416-test-e2e-aws-416
  • ci/prow/417-test-e2e-aws-417
  • pull-ci-openshift-knative-serving-release-next-413-test-e2e-aws-413
  • pull-ci-openshift-knative-serving-release-next-416-images
  • pull-ci-openshift-knative-serving-release-next-416-test-e2e-aws-416
  • pull-ci-openshift-knative-serving-release-next-417-test-e2e-aws-417
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override ?

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.

@skonto
Copy link

skonto commented Sep 24, 2024

@skonto
Copy link

skonto commented Sep 24, 2024

@ReToCode
We have an edge terminated route and obviously alpn is empty. Haproxy has this: https://github.com/openshift/router/blob/master/images/router/haproxy/conf/haproxy-config.template#L739.
Setting appProtocol: h2c for the services would fix the issue 🤞 .

As a side note, we had a problem with appProtocol: kubernetes.io/h2c and mtls (thus we have a patch that reverts the upstream stuff), see https://issues.redhat.com/browse/SRVKS-1232. However I think the reason was that port name and appProtocol should have the same value thus the issue in SRVKS-1232, specifically check the issue and the validator code. I think we need to revise the work upstream in knative#14809 as it does not work with mesh and we don't test with mesh upstream.

Ideally we want to keep appProtocol: kubernetes.io/h2c and file a bug for the OCP router so the latter supports it, see discussion here.

Also we can't just set the following to false as it will be removed in future releases (maybe just for now):

	ALTSMaxConcurrentHandshakes = uint64FromEnv("GRPC_ALTS_MAX_CONCURRENT_HANDSHAKES", 100, 1, 100)
	// EnforceALPNEnabled is set if TLS connections to servers with ALPN disabled
	// should be rejected. The HTTP/2 protocol requires ALPN to be enabled, this
	// option is present for backward compatibility. This option may be overridden
	// by setting the environment variable "GRPC_ENFORCE_ALPN_ENABLED" to "true"
	// or "false".
	EnforceALPNEnabled = boolFromEnv("GRPC_ENFORCE_ALPN_ENABLED", true)

@skonto
Copy link

skonto commented Sep 24, 2024

Reverting passes see: #893.

@ReToCode
Copy link
Member

Hm nice find. To be honest, no idea. Could we probably also just set GRPC_ENFORCE_ALPN_ENABLED? So we could keep the upstream changes?

@skonto
Copy link

skonto commented Sep 25, 2024

Ok checking in #894. I will talk to router folks to open a bug as well. Going forward we need this, setting the env var is temporary unless we patch grpc itself and remove that alpn check (a bit ugly).

Copy link

openshift-ci bot commented Sep 26, 2024

@serverless-qe: 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/417-test-e2e-aws-417 2155411 link true /test 417-test-e2e-aws-417
ci/prow/413-test-e2e-aws-413 2155411 link true /test 413-test-e2e-aws-413

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.

@ReToCode
Copy link
Member

/retest

@skonto
Copy link

skonto commented Sep 26, 2024

@ReToCode I suspect it does not get the fix, probably we will have to close it and wait for the next one?

@skonto
Copy link

skonto commented Sep 26, 2024

Wait it seems the errors are not related.

The ImageStreamTag "ubi-minimal:latest" is invalid: from: Error resolving ImageStreamTag ubi-minimal:latest in namespace openshift-marketplace: unable to find latest tagged image

@serverless-qe
Copy link
Collaborator Author

OCF Webhook is merging this PR

@serverless-qe serverless-qe merged commit 94a32b9 into release-next Sep 26, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants