-
Notifications
You must be signed in to change notification settings - Fork 0
Feat adopt Knative CE Operator #82
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughReplaced Alauda AI Model Serving Operator references with Knative CE Operator across installation and overview docs; updated package/env names, installation steps, KnativeServing YAML, networking guidance (Kourier default), and removed Istio prerequisites. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/en/installation/ai-cluster.mdx`:
- Around line 233-255: Fix the grammar typo in the sentence "Install Knative CE
Operator window will popup." located under the "### 1. Installing the Knative CE
Operator" section by changing "popup" to the two-word form "pop up" so the
sentence reads "Install Knative CE Operator window will pop up."; update any
identical occurrences of "popup" in nearby sentences (e.g., the "Install Knative
CE Operator" window lines) to the correct two-word form.
- Around line 269-329: In the "Creating Knative Serving Instance" section update
the numbered step that currently reads "7.Click **Create**." to include a space
after the period so it reads "7. Click **Create**." — locate the step under the
Steps list in the KnativeServing procedure (the line containing "7.Click
**Create**.") and insert the missing space.
🧹 Nitpick comments (1)
docs/en/installation/ai-cluster.mdx (1)
112-120: Reduce repetitive “is the” phrasing in callouts.Small wording tweak improves readability.
♻️ Suggested wording
-6. `${KNATIVE_CE_OPERATOR_PKG_NAME}` is the path to the Knative CE Operator package tarball. -7. `${REGISTRY_ADDRESS}` is the address of the external registry. -8. `${REGISTRY_USERNAME}` is the username of the external registry. -9. `${REGISTRY_PASSWORD}` is the password of the external registry. +6. `${KNATIVE_CE_OPERATOR_PKG_NAME}`: path to the Knative CE Operator package tarball. +7. `${REGISTRY_ADDRESS}`: external registry address. +8. `${REGISTRY_USERNAME}`: external registry username. +9. `${REGISTRY_PASSWORD}`: external registry password.
Deploying alauda-ai with
|
| Latest commit: |
b237383
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://16dcfeab.alauda-ai.pages.dev |
| Branch Preview URL: | https://add-knative-ce-operator.alauda-ai.pages.dev |
Signed-off-by: Yuan Fang <yuanfang@alauda.io>
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/en/installation/ai-cluster.mdx`:
- Around line 238-240: Clarify whether the istio-system namespace is provisioned
by the core/base deployment or not: update the text around the mentions of
"istio-system" and the Knative installation note (references: "istio-system",
"Knative CE Operator", "Kourier") to state explicitly whether the base cluster
still creates istio-system for legacy certificate storage, or, if Istio is not
installed, specify the alternative namespace users should use for storing custom
certificates (and change the guidance at lines that reference storing certs in
istio-system to point to that alternative). Ensure a single clear sentence near
the Knative note indicates if istio-system is present by default or only present
when Istio is installed and give the recommended namespace to use for custom
certs when using Kourier.
🧹 Nitpick comments (1)
docs/en/installation/ai-cluster.mdx (1)
287-314: Add a short security caution for enabling host-level podspec features.The YAML enables hostNetwork/hostIPC and other podspec features, which materially expands workload privileges. A brief warning helps admins understand the security implications and scope it to trusted workloads/namespaces.
💡 Suggested doc addition
@@ 6. Replace the content with the following YAML: 7. Click **Create**. + :::warning + The features below enable host-level podspec capabilities (e.g., hostNetwork/hostIPC). Use only for trusted workloads/namespaces and follow your cluster security policies. + ::: + ```yaml apiVersion: operator.knative.dev/v1beta1 kind: KnativeServing
| :::info | ||
| Starting from **Knative CE Operator**, the Knative networking layer switches to **Kourier**, so installing **Istio** is no longer required. | ||
| ::: |
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.
🧩 Analysis chain
🏁 Script executed:
wc -l docs/en/installation/ai-cluster.mdxRepository: alauda/aml-docs
Length of output: 98
🏁 Script executed:
# Read the file around lines 238-240 and search for istio-system references
cat -n docs/en/installation/ai-cluster.mdx | head -300 | tail -100Repository: alauda/aml-docs
Length of output: 3754
🏁 Script executed:
# Search for all istio-system references in the file
rg "istio-system" docs/en/installation/ai-cluster.mdx -B 2 -A 2Repository: alauda/aml-docs
Length of output: 433
🏁 Script executed:
rg "istio-system" docs/en/installation/ai-cluster.mdx -B 5 -A 5 -nRepository: alauda/aml-docs
Length of output: 802
🏁 Script executed:
rg -i "istio" docs/en/installation/ai-cluster.mdx -B 2 -A 2 -nRepository: alauda/aml-docs
Length of output: 630
Clarify whether istio-system namespace is created in the base deployment.
Lines 199–200 reference storing certificates in the istio-system namespace during initial setup, but lines 238–240 state Istio is no longer required for Knative. Clarify whether istio-system is still provisioned as part of the core cluster setup, or if an alternative namespace should be used for custom certificates when Istio is not installed.
🤖 Prompt for AI Agents
In `@docs/en/installation/ai-cluster.mdx` around lines 238 - 240, Clarify whether
the istio-system namespace is provisioned by the core/base deployment or not:
update the text around the mentions of "istio-system" and the Knative
installation note (references: "istio-system", "Knative CE Operator", "Kourier")
to state explicitly whether the base cluster still creates istio-system for
legacy certificate storage, or, if Istio is not installed, specify the
alternative namespace users should use for storing custom certificates (and
change the guidance at lines that reference storing certs in istio-system to
point to that alternative). Ensure a single clear sentence near the Knative note
indicates if istio-system is present by default or only present when Istio is
installed and give the recommended namespace to use for custom certs when using
Kourier.
Adopt Knative CE Operator(supported by Alauda Middleware team) for system integration
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.