Skip to content

Conversation

@kaldesai
Copy link

@kaldesai kaldesai commented Nov 4, 2025

Affected versions: serverless-docs-1.37

Tracking JIRAs:

Doc preview:

Reviews:

  • QE has approved this change.
  • SME has approved this change.
  • Peer has approved this change.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 4, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 4, 2025

@kaldesai: This pull request references SRVCOM-3999 which is a valid jira issue.

In response to this:

Affected versions: serverless-docs-1.37

Tracking JIRAs:

Doc preview:

Reviews:

  • QE has approved this change.
  • SME has approved this change.
  • Peer has approved this change.

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.

@kaldesai
Copy link
Author

kaldesai commented Nov 4, 2025

/label serverless

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 4, 2025

@kaldesai: This pull request references SRVCOM-3999 which is a valid jira issue.

In response to this:

Affected versions: serverless-docs-1.37

Tracking JIRAs:

Doc preview:

Reviews:

  • QE has approved this change.
  • SME has approved this change.
  • Peer has approved this change.

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 added serverless Label for all Serverless PRs size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 4, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 4, 2025

@kaldesai: This pull request references SRVCOM-3999 which is a valid jira issue.

In response to this:

Affected versions: serverless-docs-1.37

Tracking JIRAs:

Doc preview:

Reviews:

  • QE has approved this change.
  • SME has approved this change.
  • Peer has approved this change.

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.

name: knative-serving
namespace: knative-serving
annotations:
serverless.openshift.io/disable-istio-net-policies-generation: "true"
Copy link

Choose a reason for hiding this comment

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

@mvinkler could you pls verify if the setup can work without this annotation?

Copy link

Choose a reason for hiding this comment

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

I'm having some second thoughts, it isn't needed actually.

Choose a reason for hiding this comment

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

I tried without the annotation and the revision for the service never got Ready

Choose a reason for hiding this comment

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

That makes me wonder if it is needed for KnativeEventing and KnativeKafka as well. I checked for KnativeServing, three policies were created: allow-from-openshift-monitoring-ns, net-istio-webhook, webhook


* If you want to use any domain name, including those which are not subdomains of the default {ocp-product-title} cluster domain, you must set up domain mapping for those domains. For more information, see the {ServerlessProductName} documentation about xref:../../knative-serving/config-custom-domains/create-domain-mapping.adoc#serverless-create-domain-mapping_create-domain-mapping[Creating a custom domain mapping].

include::modules/serverless-ossm-external-certs.adoc[leveloffset=+1]

Choose a reason for hiding this comment

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

This module expect istio-system ns to exist in the step 4 but it is not in the prerequisities.

Choose a reason for hiding this comment

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

Also I believe the wildcart certs should be created in knative-serving-ingress namespace, WDYT @dsimansk ?

Choose a reason for hiding this comment

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

I checked and the secret needs to be in the knative-serving-ingress namespace for SM3

Choose a reason for hiding this comment

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

Yes, thanks for pointing it out and checking.

[role="_abstract"]
Before installing and configuring the {SMProductShortName} integration with {ServerlessProductShortName}, verify that the prerequisites have been met.

.Procedure

Choose a reason for hiding this comment

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

Check whether {SMProductName} istio-ingressgateway is exposed as type NodePort or LoadBalancer: section: istio-ingressgateway object does not exist at this point (we consider clean installation in this document?)

Copy link
Author

Choose a reason for hiding this comment

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

Do you want me to remove that step then?

Choose a reason for hiding this comment

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

@dsimansk I believe this prerequisite check makes no sense as you are just about to install SM, so why checking for istio-ingressgateway object in ns which doesn't exist at this point at all?

Choose a reason for hiding this comment

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

Since @dsimansk did not have any input on this, let's just delete this prerequisity step.

Choose a reason for hiding this comment

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

Yes, I believe it's meant to identify a potential conflict of pre-existing Service Mesh instance on the cluster. I agree we can assume a clean installation for now.

name: knative-serving
namespace: knative-serving
annotations:
serverless.openshift.io/disable-istio-net-policies-generation: "true"

Choose a reason for hiding this comment

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

I tried without the annotation and the revision for the service never got Ready

+
[source,terminal]
----
Hello Serverless!

Choose a reason for hiding this comment

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

With the multiarch image, the output is actually
{"artifact":"knative-showcase","greeting":"Welcome"}

Copy link
Author

Choose a reason for hiding this comment

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

I have made the changes accodingly

@maschmid
Copy link
Contributor

For ServiceMesh 3, KnativeEventing also needs the

  annotations:
    serverless.openshift.io/disable-istio-net-policies-generation: "true"

(the same one that is on KnativeServing)

@kaldesai
Copy link
Author

@maschmid @mvinkler I have added your suggestions. Can you please check now?

@openshift-ci
Copy link

openshift-ci bot commented Nov 18, 2025

@kaldesai: all tests passed!

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.

@dsimansk
Copy link

@kaldesai it looks good to me. I've added my comments to Michal's questions or concerns, but overall it's ready.

@mvinkler @maschmid thanks for the inputs and additional corrections.

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

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. serverless Label for all Serverless PRs size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants