update docs: use operator to deploy eloqdoc#242
update docs: use operator to deploy eloqdoc#242starrysky9959 wants to merge 3 commits intoeloqdata:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive documentation for deploying EloqDoc on AWS EKS using the EloqDoc Operator, providing a declarative and simplified deployment approach. The guide covers the complete setup process from cluster creation to testing the deployment.
Key Changes:
- Added a new deployment guide documenting the operator-based installation method
- Updated the main README to reference the new operator deployment guide
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/deploy-with-operator.md | New comprehensive guide covering EKS cluster setup, operator installation, and EloqDoc deployment with detailed S3 configuration |
| README.md | Added link to the new operator-based deployment guide |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| memory: "512Mi" | ||
| cpu: "1" | ||
| keySpaceName: e2e | ||
| image: eloqdata/eloqdoc-rocks-cloud:release-0.2.6 |
There was a problem hiding this comment.
The documentation does not explain whether 'release-0.2.6' is a placeholder or an actual version. Consider adding a note about checking for the latest available image version or clarifying if this is an example value that should be updated.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds operator-based deployment documentation and links: new docs under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant HelmKubectl as Helm/kubectl
participant EloqOperator as Eloq Operator
participant K8s as Kubernetes API
participant Storage as External Storage (S3/BOS/EBS)
rect `#f0f7ff`
User->>HelmKubectl: apply Helm chart / CRs
HelmKubectl->>K8s: create resources (CR, Secrets, Deployments)
K8s->>EloqOperator: notify of new CR
EloqOperator->>K8s: reconcile -> create StatefulSets, Services, PVCs
EloqOperator->>Storage: configure/validate external storage (S3/BOS/EBS)
Storage-->>EloqOperator: storage ready/credentials validated
EloqOperator-->>K8s: update CR status (Ready)
K8s-->>User: resources available / service endpoint
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🪛 LanguageTooldocs/deploy-with-operator/aws-eks.md[grammar] ~613-~613: The word ‘Install’ is not a noun. (A_INSTALL) [grammar] ~633-~633: The word ‘Install’ is not a noun. (A_INSTALL) [uncategorized] ~792-~792: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [typographical] ~923-~923: If specifying a range, consider using an en dash instead of a hyphen. (HYPHEN_TO_EN) README.md[style] ~196-~196: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) docs/deploy-with-operator/baidu-cce.md[uncategorized] ~10-~10: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL) [uncategorized] ~310-~310: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA) [grammar] ~312-~312: “values” is a plural noun. It appears that the verb form is incorrect. (PCT_PLURAL_NOUN_SINGULAR_VERB_AGREEMENT) [uncategorized] ~527-~527: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) [typographical] ~576-~576: If specifying a range, consider using an en dash instead of a hyphen. (HYPHEN_TO_EN) 🪛 markdownlint-cli2 (0.18.1)docs/deploy-with-operator/aws-eks.md802-802: Fenced code blocks should have a language specified (MD040, fenced-code-language) docs/deploy-with-operator/README.md73-73: Fenced code blocks should have a language specified (MD040, fenced-code-language) docs/deploy-with-operator/baidu-cce.md538-538: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🔇 Additional comments (1)
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/deploy-with-operator.md (1)
904-905: Clarify the image tag usage
image: eloqdata/eloqdoc-rocks-cloud:release-0.2.6reads like an example but might go stale quickly. Please add guidance to check for the latest published tag (or note that 0.2.6 is current) so users don't deploy an outdated build.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)docs/deploy-with-operator.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/deploy-with-operator.md
[grammar] ~12-~12: There might be a mistake here.
Context: ...ater) - helm installed (v3.0 or later) - An AWS account with permissions to creat...
(QB_NEW_EN)
[grammar] ~258-~258: There might be a mistake here.
Context: ...e:** Replace the following placeholders: > - <YOUR_ACCOUNT_ID>: Your AWS account ID > - `<YOUR_IAM_USE...
(QB_NEW_EN)
[grammar] ~259-~259: There might be a mistake here.
Context: ...<YOUR_ACCOUNT_ID>: Your AWS account ID > - <YOUR_IAM_USER>: Your IAM username > - Adjust the `regi...
(QB_NEW_EN)
[grammar] ~260-~260: There might be a mistake here.
Context: ...> - <YOUR_IAM_USER>: Your IAM username > - Adjust the region, availabilityZones...
(QB_NEW_EN)
[grammar] ~263-~263: There might be a mistake here.
Context: ...nts The bootstrap script automatically: - Detects and formats attached instance st...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...attached instance store disks (≥800 GiB) - Creates an XFS filesystem with quota sup...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ...tes an XFS filesystem with quota support - Mounts the disk to /mnt/xfs-quota - Ma...
(QB_NEW_EN)
[grammar] ~835-~835: There might be a mistake here.
Context: ...Base name for the transaction log bucket - objectStoreBucketName: Base name for the object store bucket ...
(QB_NEW_EN)
[grammar] ~836-~836: There might be a mistake here.
Context: ...t (can be the same as txLogBucketName) - bucketPrefix: Prefix that will be prepended to bucke...
(QB_NEW_EN)
[grammar] ~837-~837: There might be a mistake here.
Context: ...x that will be prepended to bucket names - txLogObjectPath: Path prefix for transaction logs withi...
(QB_NEW_EN)
[grammar] ~838-~838: There might be a mistake here.
Context: ...x for transaction logs within the bucket - objectStoreObjectPath: Path prefix for object store data with...
(QB_NEW_EN)
[grammar] ~839-~839: There might be a mistake here.
Context: ... for object store data within the bucket - region: AWS region where buckets will be creat...
(QB_NEW_EN)
[grammar] ~858-~858: There might be a mistake here.
Context: ...3-store ``` The created bucket will be: - Bucket name: eloqdoc-my-cluster-data ...
(QB_NEW_EN)
[grammar] ~861-~861: There might be a mistake here.
Context: ...this bucket, data is organized by paths: - Transaction logs: `s3://eloqdoc-my-clust...
(QB_NEW_EN)
[grammar] ~862-~862: There might be a mistake here.
Context: ...organized by paths: - Transaction logs: s3://eloqdoc-my-cluster-data/eloqdoc-rocksdb-s3-log/ - Object store data: `s3://eloqdoc-my-clus...
(QB_NEW_EN)
[grammar] ~952-~952: There might be a mistake here.
Context: ...> Note: Update the following values: > - <YOUR_S3_BUCKET_BASE_NAME>: Base name for S3 buckets (e.g., `my-el...
(QB_NEW_EN)
[grammar] ~955-~955: There might be a mistake here.
Context: ... actual S3 bucket names created will be: > - Transaction log bucket: `<...
(QB_NEW_EN)
[grammar] ~956-~956: There might be a mistake here.
Context: ...Name>(e.g.,eloqdoc-my-eloqdoc-data) > - Object store bucket: <obj...
(QB_NEW_EN)
[grammar] ~957-~957: There might be a mistake here.
Context: ...Name>(e.g.,eloqdoc-my-eloqdoc-data`) > - If using the same bucket name, the data ...
(QB_NEW_EN)
[grammar] ~958-~958: There might be a mistake here.
Context: ...ket name, the data will be organized as: > - Transaction logs: `s3://<b...
(QB_NEW_EN)
[grammar] ~959-~959: There might be a mistake here.
Context: ...e organized as: > - Transaction logs: s3://<bucketPrefix><bucketName>/<txLogObjectPath>/ > - Object store: `s3://<bucke...
(QB_NEW_EN)
[grammar] ~960-~960: There might be a mistake here.
Context: .../> - Object store:s3:////` > - The buckets will be **automatically crea...
(QB_NEW_EN)
[grammar] ~964-~964: There might be a mistake here.
Context: ...ucket names comply with S3 naming rules: - Must be globally unique across all AWS a...
(QB_NEW_EN)
[grammar] ~1081-~1081: There might be a mistake here.
Context: ...stance to the public internet. Consider: > - Using security groups to restrict access...
(QB_NEW_EN)
[grammar] ~1082-~1082: There might be a mistake here.
Context: ...restrict access to specific IP addresses > - Implementing network policies > - Using ...
(QB_NEW_EN)
[grammar] ~1083-~1083: There might be a mistake here.
Context: ...resses > - Implementing network policies > - Using a VPN or AWS PrivateLink for produ...
(QB_NEW_EN)
| - name: ap-northeast-1a-i4i-xlarge | ||
| privateNetworking: true | ||
| availabilityZones: ['ap-northeast-1a'] | ||
| instanceType: i4i.xlarge | ||
| spot: false | ||
| volumeSize: 50 | ||
| ami: ami-0421a6503852f2cdb | ||
| amiFamily: Ubuntu2204 | ||
| labels: | ||
| xfsQuota: enabled | ||
| minSize: 0 | ||
| desiredCapacity: 0 | ||
| maxSize: 3 | ||
|
|
There was a problem hiding this comment.
Ensure node group actually provisions worker nodes
desiredCapacity: 0 means this node group comes up empty. Every later step (installing CSI drivers, operator, EloqDoc pods) requires schedulable nodes, so the walkthrough as written stalls immediately after cluster creation. Bump desiredCapacity (and usually minSize) to at least 1, or call out an explicit eksctl scale nodegroup step right after creation so users get usable capacity.
🤖 Prompt for AI Agents
In docs/deploy-with-operator.md around lines 35 to 48, the nodegroup is created
with desiredCapacity: 0 which leaves no schedulable worker nodes and causes
subsequent steps to stall; update the nodegroup configuration to set
desiredCapacity (and typically minSize) to at least 1, or add an explicit eksctl
scale nodegroup step immediately after cluster creation to ensure at least one
worker is provisioned before installing CSI drivers, the operator, and pods.
| cat > EloqDBResourceIAMPolicy.json <<EOF | ||
| { | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
| { | ||
| "Sid": "S3Access", | ||
| "Effect": "Allow", | ||
| "Action": "s3:*", | ||
| "Resource": "arn:aws:s3:::*" | ||
| }, | ||
| { | ||
| "Sid": "EC2Permissions", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "ec2:DescribeSubnets", | ||
| "ec2:DescribeNetworkInterfaces", | ||
| "ec2:CreateNetworkInterface" | ||
| ], | ||
| "Resource": "*" | ||
| }, | ||
| { | ||
| "Sid": "EKSAccess", | ||
| "Effect": "Allow", | ||
| "Action": [ | ||
| "eks:DescribeCluster" | ||
| ], | ||
| "Resource": "*" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Tighten the S3 permissions
Granting s3:* on arn:aws:s3:::* hands the EloqDoc role full control over every bucket in the account, which is far beyond what the operator needs and violates least-privilege guidance. Please scope the actions down (e.g., to s3:GetObject, PutObject, ListBucket, CreateBucket, etc.) and restrict the resources to the specific bucket ARNs/prefixes created for this deployment.
🤖 Prompt for AI Agents
In docs/deploy-with-operator.md around lines 752 to 781, the IAM policy
currently grants "s3:*" on "arn:aws:s3:::*" which is overly broad; replace the
wildcard S3 permission with a minimal set of actions the operator actually needs
(for example: "s3:ListBucket", "s3:GetObject", "s3:PutObject", "s3:DeleteObject"
and optionally "s3:CreateBucket" only if the operator must create buckets) and
tighten the Resource ARNs to the exact bucket(s) and object prefixes used by
this deployment (use the bucket ARN for ListBucket and the bucket/object/* ARN
for Get/Put/Delete), optionally add condition keys (e.g., aws:SourceAccount or
aws:Referer) if applicable, and remove the global "arn:aws:s3:::*" entry to
adhere to least-privilege principles.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/deploy-with-operator.md (2)
35-48: Provision at least one worker node.
minSize: 0anddesiredCapacity: 0leave the cluster without any schedulable workers, so every subsequent install step (CSI driver, Operator, EloqDoc pods) stalls. SetminSize/desiredCapacity≥ 1 or document an immediateeksctl scale nodegroupstep to bring up capacity.
711-720: Tighten the S3 IAM policy scope.
s3:*onarn:aws:s3:::*gives the service account full control over every bucket in the account. Please replace it with the specific S3 actions EloqDoc needs (e.g.,ListBucket,GetObject,PutObject,DeleteObject,CreateBucketif necessary) and scopeResourceto the actual bucket ARN(s) and prefixes created for this deployment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/deploy-with-operator.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/deploy-with-operator.md
[grammar] ~12-~12: There might be a mistake here.
Context: ...ater) - helm installed (v3.0 or later) - An AWS account with permissions to creat...
(QB_NEW_EN)
[grammar] ~258-~258: There might be a mistake here.
Context: ...e:** Replace the following placeholders: > - <YOUR_ACCOUNT_ID>: Your AWS account ID > - `<YOUR_IAM_USE...
(QB_NEW_EN)
[grammar] ~259-~259: There might be a mistake here.
Context: ...<YOUR_ACCOUNT_ID>: Your AWS account ID > - <YOUR_IAM_USER>: Your IAM username > - Adjust the `regi...
(QB_NEW_EN)
[grammar] ~260-~260: There might be a mistake here.
Context: ...> - <YOUR_IAM_USER>: Your IAM username > - Adjust the region, availabilityZones...
(QB_NEW_EN)
[grammar] ~263-~263: There might be a mistake here.
Context: ...nts The bootstrap script automatically: - Detects and formats attached instance st...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...attached instance store disks (≥800 GiB) - Creates an XFS filesystem with quota sup...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ...tes an XFS filesystem with quota support - Mounts the disk to /mnt/xfs-quota - Ma...
(QB_NEW_EN)
[grammar] ~266-~266: There might be a mistake here.
Context: ...with quota support - Mounts the disk to /mnt/xfs-quota - Makes the mount persistent across reboot...
(QB_NEW_EN)
[grammar] ~794-~794: There might be a mistake here.
Context: ...Base name for the transaction log bucket - objectStoreBucketName: Base name for the object store bucket ...
(QB_NEW_EN)
[grammar] ~795-~795: There might be a mistake here.
Context: ...t (can be the same as txLogBucketName) - bucketPrefix: Prefix that will be prepended to bucke...
(QB_NEW_EN)
[grammar] ~796-~796: There might be a mistake here.
Context: ...x that will be prepended to bucket names - txLogObjectPath: Path prefix for transaction logs withi...
(QB_NEW_EN)
[grammar] ~797-~797: There might be a mistake here.
Context: ...x for transaction logs within the bucket - objectStoreObjectPath: Path prefix for object store data with...
(QB_NEW_EN)
[grammar] ~798-~798: There might be a mistake here.
Context: ... for object store data within the bucket - region: AWS region where buckets will be creat...
(QB_NEW_EN)
[grammar] ~817-~817: There might be a mistake here.
Context: ...3-store ``` The created bucket will be: - Bucket name: eloqdoc-my-cluster-data ...
(QB_NEW_EN)
[grammar] ~820-~820: There might be a mistake here.
Context: ...this bucket, data is organized by paths: - Transaction logs: `s3://eloqdoc-my-clust...
(QB_NEW_EN)
[grammar] ~821-~821: There might be a mistake here.
Context: ...organized by paths: - Transaction logs: s3://eloqdoc-my-cluster-data/eloqdoc-rocksdb-s3-log/ - Object store data: `s3://eloqdoc-my-clus...
(QB_NEW_EN)
[grammar] ~911-~911: There might be a mistake here.
Context: ...> Note: Update the following values: > - <YOUR_S3_BUCKET_BASE_NAME>: Base name for S3 buckets (e.g., `my-el...
(QB_NEW_EN)
[grammar] ~914-~914: There might be a mistake here.
Context: ... actual S3 bucket names created will be: > - Transaction log bucket: `<...
(QB_NEW_EN)
[grammar] ~915-~915: There might be a mistake here.
Context: ...Name>(e.g.,eloqdoc-my-eloqdoc-data) > - Object store bucket: <obj...
(QB_NEW_EN)
[grammar] ~916-~916: There might be a mistake here.
Context: ...Name>(e.g.,eloqdoc-my-eloqdoc-data`) > - If using the same bucket name, the data ...
(QB_NEW_EN)
[grammar] ~917-~917: There might be a mistake here.
Context: ...ket name, the data will be organized as: > - Transaction logs: `s3://<b...
(QB_NEW_EN)
[grammar] ~918-~918: There might be a mistake here.
Context: ...e organized as: > - Transaction logs: s3://<bucketPrefix><bucketName>/<txLogObjectPath>/ > - Object store: `s3://<bucke...
(QB_NEW_EN)
[grammar] ~919-~919: There might be a mistake here.
Context: .../> - Object store:s3:////` > - The buckets will be **automatically crea...
(QB_NEW_EN)
[grammar] ~923-~923: There might be a mistake here.
Context: ...ucket names comply with S3 naming rules: - Must be globally unique across all AWS a...
(QB_NEW_EN)
[grammar] ~1040-~1040: There might be a mistake here.
Context: ...stance to the public internet. Consider: > - Using security groups to restrict access...
(QB_NEW_EN)
[grammar] ~1041-~1041: There might be a mistake here.
Context: ...restrict access to specific IP addresses > - Implementing network policies > - Using ...
(QB_NEW_EN)
[grammar] ~1042-~1042: There might be a mistake here.
Context: ...resses > - Implementing network policies > - Using a VPN or AWS PrivateLink for produ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
docs/deploy-with-operator.md
804-804: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
165164e to
edc4c36
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
docs/deploy-with-operator/aws-eks.md (1)
718-719: Document the S3 IAM policy to comply with AWS security best practices and scope permissions appropriately.This documentation currently instructs users to create an IAM policy with
"s3:*"on"arn:aws:s3:::*", which grants full S3 control across the entire AWS account. AWS security best practices explicitly recommend avoiding broad S3:* permissions and instead scoping by specific resource ARNs with minimal required actions.Since no disclaimer indicates this is temporary demo-only configuration, users deploying from this guide will provision overly permissive production IAM policies. The documentation should either:
- Add a prominent security warning clarifying this is for demonstration purposes, or
- Provide the hardened policy directly, as proposed:
{ "Sid": "S3Access", "Effect": "Allow", - "Action": "s3:*", - "Resource": "arn:aws:s3:::*" + "Action": [ + "s3:ListBucket", + "s3:GetObject", + "s3:PutObject", + "s3:DeleteObject", + "s3:CreateBucket" + ], + "Resource": [ + "arn:aws:s3:::eloqdoc-*", + "arn:aws:s3:::eloqdoc-*/*" + ] },This restricts EloqDoc to buckets prefixed with
eloqdoc-and grants only necessary S3 actions (list, get, put, delete, create).
🧹 Nitpick comments (1)
README.md (1)
196-196: Consider rewording to avoid successive "Follow" phrases.Three consecutive bullet points now begin with "Follow" (lines 194–196). For readability, consider rephrasing at least one to add variety.
Possible alternative:
-* Follow [Kubernetes deployment guide](docs/deploy-with-operator/README.md) to learn how to deploy EloqDoc on managed Kubernetes services (AWS EKS, Baidu CCE) using the Eloq Operator. +* Deploy EloqDoc on managed Kubernetes services (AWS EKS, Baidu CCE) using the Eloq Operator—see the [Kubernetes deployment guide](docs/deploy-with-operator/README.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)docs/deploy-with-operator/README.md(1 hunks)docs/deploy-with-operator/aws-eks.md(1 hunks)docs/deploy-with-operator/baidu-cce.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/deploy-with-operator/baidu-cce.md
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...installed (v3+) ## Quick overview (high level steps) 1. Create a CCE cluster. 2....
(QB_NEW_EN_HYPHEN)
README.md
[style] ~196-~196: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...learn major configuration parameters. * Follow [Kubernetes deployment guide](docs/depl...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
docs/deploy-with-operator/aws-eks.md
804-804: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/deploy-with-operator/baidu-cce.md
553-553: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/deploy-with-operator/README.md
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
docs/deploy-with-operator/README.md (1)
1-133: Well-structured deployment overview and hub document.This new README serves as an effective entry point for operator-based deployments. The overview section clearly explains the operator's role, deployment architecture is well-illustrated, and navigation to platform-specific guides (AWS EKS, Baidu CCE) is intuitive. Storage configuration section provides helpful context for understanding the hybrid approach.
| **Bucket Naming Convention:** | ||
|
|
||
| The actual S3 bucket names are formed by combining the prefix and base name: | ||
| ``` |
There was a problem hiding this comment.
Specify language for fenced code block.
The code block at Line 804 should declare its language for syntax highlighting.
-```
+```
Actual Bucket Name = bucketPrefix + bucketNameCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
804-804: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/deploy-with-operator/aws-eks.md around line 804, the fenced code block
is missing a language specifier; update the opening fence to include an
appropriate language (for this non-executable example use "text" or "bash") so
it reads like ```text or ```bash to enable syntax highlighting and improve
readability.
| - kubectl installed and configured locally (v1.28+ recommended) | ||
| - helm installed (v3+) | ||
|
|
||
| ## Quick overview (high level steps) |
There was a problem hiding this comment.
Fix hyphenation in "high level steps" heading.
Line 10 should use a hyphen when "high level" modifies "steps" as a compound adjective.
-## Quick overview (high level steps)
+## Quick overview (high-level steps)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Quick overview (high level steps) | |
| ## Quick overview (high-level steps) |
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...installed (v3+) ## Quick overview (high level steps) 1. Create a CCE cluster. 2....
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In docs/deploy-with-operator/baidu-cce.md around line 10, the heading "Quick
overview (high level steps)" needs hyphenation; change it to "Quick overview
(high-level steps)" so the compound adjective is correct.
|
|
||
| ## Deployment Architecture | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Specify language for fenced code block (ASCII diagram).
The ASCII architecture diagram at Line 67 should declare a language. Since it's a text diagram, use text or plaintext:
-```
+```text
┌─────────────────────────────────────────────────────────┐🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/deploy-with-operator/README.md around line 67 the fenced code block
containing an ASCII architecture diagram does not declare a language; update the
opening fence to include a language identifier (e.g., ```text or ```plaintext)
so the diagram is treated as plain text and rendered/processed correctly by
markdown parsers and syntax highlighters.
edc4c36 to
e23c73f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/deploy-with-operator/baidu-cce.md (1)
10-10: Fix hyphenation in compound adjective.The heading should use a hyphen to join the compound adjective "high-level" that modifies "steps":
-## Quick overview (high level steps) +## Quick overview (high-level steps)
🧹 Nitpick comments (1)
README.md (1)
196-196: Reduce repetition: Consider rewording to vary sentence starters.Lines 193–196 all begin with "Follow," which creates repetitive phrasing. Since this is the fourth consecutive "Follow" bullet, consider using a synonym or restructuring one or two of these to improve readability.
Example alternatives:
- "Consult the Kubernetes deployment guide"
- "Use the Kubernetes deployment guide"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)docs/deploy-with-operator/README.md(1 hunks)docs/deploy-with-operator/aws-eks.md(1 hunks)docs/deploy-with-operator/baidu-cce.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/deploy-with-operator/README.md
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~196-~196: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...learn major configuration parameters. * Follow [Kubernetes deployment guide](docs/depl...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/deploy-with-operator/baidu-cce.md
[grammar] ~10-~10: Use a hyphen to join words.
Context: ...installed (v3+) ## Quick overview (high level steps) 1. Create a CCE cluster. 2....
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/deploy-with-operator/aws-eks.md
804-804: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/deploy-with-operator/baidu-cce.md
541-541: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| minSize: 0 | ||
| desiredCapacity: 0 |
There was a problem hiding this comment.
❓ Verification inconclusive
This issue was previously flagged and remains unresolved. With minSize: 0 and desiredCapacity: 0, no worker nodes will be provisioned, causing all subsequent installation steps (CSI drivers, operator, pods) to stall immediately after cluster creation.
Set desiredCapacity: 1 (and typically minSize: 1) to provision at least one schedulable node:
minSize: 0
- desiredCapacity: 0
+ desiredCapacity: 1
maxSize: 3Alternatively, add an explicit scale-up step after cluster creation:
eksctl scale nodegroup --cluster eloqdb-demo --name ap-northeast-1a-i4i-xlarge --nodes 1I need to verify the actual state of the file to validate this review comment. Let me inspect the relevant section:
Let me verify the actual state of the file:
With desiredCapacity: 0, no EC2 instances will launch initially. There is an AWS incompatibility with cluster-autoscaler: while you can scale a managed node group down to zero, you cannot scale it back up without a workaround. The workaround requires special tags (e.g., k8s.io/cluster-autoscaler/node-template/label/...) for cluster-autoscaler to detect and scale from zero.
Unless your guide explicitly provides post-cluster-creation scaling instructions (manual scaling, Cluster Autoscaler with tags, or Capacity Blocks), users will be unable to provision worker nodes to deploy the operator and CSI drivers.
Either:
- Set
desiredCapacity: 1andminSize: 1to provision initial nodes, or - Document the explicit scaling step after cluster creation with required tags/configuration
🤖 Prompt for AI Agents
In docs/deploy-with-operator/aws-eks.md around lines 45-46, the node group is
configured with minSize: 0 and desiredCapacity: 0 which prevents any EC2 worker
nodes from launching and can block initial deployment; change the values to
either minSize: 1 and desiredCapacity: 1 to ensure at least one node is
provisioned at creation, or keep zeros but add clear documentation immediately
after these lines describing the required post-creation steps (how to manually
scale the node group or configure Cluster Autoscaler tags/Capacity Blocks to
allow scaling from zero) so users can recover and provision workers.
| ``` | ||
| Actual Bucket Name = bucketPrefix + bucketName |
There was a problem hiding this comment.
Specify language for fenced code block.
The code block at line 804 is missing a language specifier, which prevents proper syntax highlighting and violates markdown standards.
-```
+```text
Actual Bucket Name = bucketPrefix + bucketName
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
804-804: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/deploy-with-operator/aws-eks.md around lines 804 to 805, the fenced code
block lacks a language specifier; update the opening fence to include an
appropriate language tag (e.g., ```text or ```bash) so the block becomes ```text
(or chosen language) and then the content, followed by the closing ```, ensuring
proper syntax highlighting and Markdown validity.
| ``` | ||
| Actual Bucket Name = bucketPrefix + bucketName |
There was a problem hiding this comment.
Specify language for fenced code block.
The code block at line 541 is missing a language specifier, which prevents proper syntax highlighting and violates markdown standards.
-```
+```text
Actual Bucket Name = bucketPrefix + bucketName
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
541-541: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/deploy-with-operator/baidu-cce.md around lines 541 to 542, the fenced
code block lacks a language specifier; update the opening fence from ``` to
```text so it reads ```text to enable proper syntax highlighting and comply with
Markdown standards, leaving the block contents unchanged and keeping the closing
fence as ``` .
e23c73f to
ebace71
Compare
Summary by CodeRabbit