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
docs/OSUpgrades.md: Update/reword #1896
docs/OSUpgrades.md: Update/reword #1896
Conversation
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.
Overall lgtm, minor nits
@@ -14,83 +14,98 @@ These bootimages are built using [coreos-assembler](https://github.com/coreos/co | |||
Today, [the installer](https://github.com/openshift/installer/) pins the "bootimages" | |||
it uses, and released installers also pin the release image. As noted above, | |||
release images contain `machine-os-content`, which can be a *different* | |||
RHCOS version. | |||
RHCOS version. You can find the installer-pinned bootimage in e.g. [this file](https://github.com/openshift/installer/blob/release-4.4/data/data/rhcos.json). |
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.
maybe link to master instead of the release-4.4 branch
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.
agree
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.
But that file is highly likely to go away after openshift/enhancements#201 - seems more useful to have an old but valid link.
docs/OSUpgrades.md
Outdated
a daemon running on the host. | ||
|
||
The MCD tries to watch the systemd journal for relevant service and proxy logs, | ||
so you *should* be able to `oc -n openshift-machine-config-operator logs pod/machine-config-daemon-...` |
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.
should we specify oc -n openshift-machine-config-operator logs -c machine-config-daemon pod/machine-config-daemon-...
for clarity
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.
agree
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.
Some comments, this is amazing!!!
docs/OSUpgrades.md
Outdated
with the OSTree repository. | ||
The reason we wrap an OSTree commit inside a container image is so that | ||
the release image encapsulates basically everything about a cluster (except the bootimage). | ||
This makes it easy to e.g. mirror updates offline. |
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.
"easy to mirror"
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.
It's not just about mirroring though. Having a single artifact means that for example in CI we can test exactly one thing - today the RHCOS dependency tends to "leak" outside of the release image.
|
||
When the administrator starts an `oc adm upgrade`, if a new `machine-os-content` | ||
is provided in the release image, it will be rolled out to the control plane | ||
and workers. | ||
|
||
Every change now will be managed by a `machineconfigpool`, ensuring | ||
that only 1 machine at a time is changed (via `maxUnavailable: 1` default). | ||
|
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.
do we want to tell them to watch the oc get mcp
for update rollouts?
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 think the target audience for this doc is someone who understands operating systems and Kubernetes at a high level - they're just looking for an explanation of the radically novel and unique thing we've done to combine them 😄
So we should expect them to know how to oc get machineconfigpool
.
docs/OSUpgrades.md
Outdated
a daemon running on the host. | ||
|
||
The MCD tries to watch the systemd journal for relevant service and proxy logs, | ||
so you *should* be able to `oc -n openshift-machine-config-operator logs pod/machine-config-daemon-...` |
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.
agree
Various updates/clarifications here. (If anyone wants to do nontrivial edits, feel free to toss this into a hackmd.io doc and we can collaborate!)
961fd27
to
9cf72f1
Compare
Pushed an update for various bits - in the future I'll probably put up a hackmd.io so we can do this more collaboratively. |
Any blockers for landing this? |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/override ci/prow/e2e-aws-workers-rhel7 no reason to have this continue running for doc changes |
@yuqi-zhang: Overrode contexts on behalf of yuqi-zhang: ci/prow/e2e-aws-workers-rhel7 In response to this:
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/test-infra repository. |
@cgwalters: The following tests failed, say
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/test-infra repository. I understand the commands that are listed here. |
/override ci/prow/e2e-aws-scaleup-rhel7 |
@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/e2e-aws-scaleup-rhel7 In response to this:
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/test-infra repository. |
/override ci/prow/e2e-gcp-upgrade |
@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/e2e-gcp-upgrade In response to this:
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/test-infra repository. |
Various updates/clarifications here. (If anyone wants to
do nontrivial edits, feel free to toss this into a hackmd.io
doc and we can collaborate!)