-
Notifications
You must be signed in to change notification settings - Fork 20
add operator v1.3 docs #910
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
Conversation
Docs Preview
|
Docs Preview
|
Deploying documentation with
|
| Latest commit: |
384609c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b8ef65f5.documentation-21k.pages.dev |
| Branch Preview URL: | https://operator-v1-3-0.documentation-21k.pages.dev |
3787493 to
ccb76df
Compare
f80ff74 to
13ba816
Compare
|
@mackrorysd could I get a stamp of approval on these docs updates? I already had @realtonyyoung and @alexeyzimarev review the changes pr (#911). (Alexey's review was over zoom and he didn't actually press "approve", so you won't see the evidence of his review, but the last two comments on there that I authored were notes I took while talking to him.) |
docs/server/kubernetes-operator/v1.3.0/getting-started/README.md
Outdated
Show resolved
Hide resolved
docs/server/kubernetes-operator/v1.3.0/getting-started/README.md
Outdated
Show resolved
Hide resolved
docs/server/kubernetes-operator/v1.3.0/getting-started/README.md
Outdated
Show resolved
Hide resolved
docs/server/kubernetes-operator/v1.3.0/getting-started/README.md
Outdated
Show resolved
Hide resolved
| * Allow manual overrides to the generated ConfigMap that is passed to KurrentDB. Previously, if a | ||
| user manually altered the ConfigMap it would get immediately overwritten, where as now it will | ||
| "stick" until the next time the KurrentDB resources is updated. | ||
| * Fix a bug affecting the KurrentDBBackup behavior when cluster's fqdnTemplate met certain criteria. |
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.
Might be helpful to include the criteria that caused problems on previous versions?
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.
Nah. This user knows who they are. For everybody else, further detail is just noise.
docs/server/kubernetes-operator/v1.3.0/getting-started/installation.md
Outdated
Show resolved
Hide resolved
docs/server/kubernetes-operator/v1.3.0/getting-started/installation.md
Outdated
Show resolved
Hide resolved
| ``` | ||
| *Expected Output*: | ||
| ``` | ||
| customresourcedefinition.apiextensions.k8s.io/kurrentdbbackups.kubernetes.kurrent.io created |
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.
No action required: I just love including expected output!
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.
Great work, Chris (I think)
| - Deploys a new Helm release called `kurrentdb-operator` in the `kurrent` namespace | ||
|
|
||
| ::: important | ||
| Make sure the namespaces listed as part of the `operator.namespaces` parameter already exist before running the command (unless you are using the Operator to target the namespace that it will be deployed in to). |
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.
I don't think I understand this. What are you doing with the operator.namespaces field, if not to always target the namespace it will be deployed in to?
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.
Oh - it will create the one specified in --namespace, but not the others?
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.
Two parts to this answer.
- the
--set operator.namespaces='{A, B}'tells the operator which namespaces it is going to control, not which namespace to install into. I reworded to make that clearer. - I think this parenthetical statement is too in-the-weeds. It isn't exactly wrong, but it's also not quite clear enough, and when you make it fully clear it subtracts more clarity than it adds correctness. So I just removed it.
docs/server/kubernetes-operator/v1.3.0/operations/database-backup.md
Outdated
Show resolved
Hide resolved
|
|
||
| Once deployed, navigate to the [Viewing Backups](#viewing-backups) section. | ||
|
|
||
| ## Backing up a specific node |
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.
Would it be helpful to elaborate on when you would back up each node, vs. just the leader?
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.
I'm not gonna lie, I don't think this feature is written very well.
I don't know why you wouldn't just prefer the leader, since we're doing volume-level snapshotting. I imagine that means the performance cost to the leader is really small, which is the only reason I can imagine why you'd snapshot anything but the leader.
But even if the CSI implementation had really expensive volume snapshots, allowing a user to manually select which node to snapshot is a weird choice, because either you want the leader or you want something like "the follower which is lagging the least" and making the end user figure that out is terrible.
So I'm inclined to leave this section of the docs as-is.
mackrorysd
left a 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.
Thanks!
No description provided.