Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Aug 17, 2021

In 4.1, the installer used to explicitly set upstream to our default URI. But in openshift/installer#c9095b34518a0 (openshift/installer#4112), which landed in 4.7 and was not backported, I'd stopped doing that. In clusters born in 4.7 and later, the installer will leave upstream unset, and the cluster-version operator will default to making a reasonable choice.

We still need to talk about explicit upstreams in the case where folks are pointing their cluster at a local OpenShift Update Service, but this commit drops the properties where we were incidentally pointing at the default, Red-Hat-hosted location, because explicitly setting that value is an anti-pattern that makes it harder for clusters to adapt if we try to move our default location elsewhere in the future.

@netlify
Copy link

netlify bot commented Aug 17, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: aea8d69

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/6195667c58457f00073bb1ca

😎 Browse the preview: https://deploy-preview-35567--osdocs.netlify.app

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 17, 2021
Copy link
Member

@LalatenduMohanty LalatenduMohanty left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2021
Copy link
Member Author

Choose a reason for hiding this comment

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

nit: this is an indent issue, because status is a sibling of spec, not a child of spec. But orthogonal to this PR's effort, and this is in _unused_topics anyway, so punting for now.

@shellyyang1989
Copy link

@wking We have a known issue [1]. So I get it linked to the bz.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1910286

@shellyyang1989
Copy link

@wking During the PR review, we happen to find that the 4.6 cluster is still displayed in 4.7 [1] and 4.8 [2] docs. We are expecting that they have 4.7 and 4.8 cluster displayed in the docs respectively. It's unrelated to this PR, so I'm just confirming with you if you'd like to get it fixed in this PR together. If not, I'm fine to file a bz to track it. Thanks!

[1] https://docs.openshift.com/container-platform/4.7/installing/validating-an-installation.html#getting-cluster-version-and-update-details_validating-an-installation
[2] https://docs.openshift.com/container-platform/4.8/installing/validating-an-installation.html#getting-cluster-version-and-update-details_validating-an-installation

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 18, 2021

New changes are detected. LGTM label has been removed.

@wking wking force-pushed the drop-explicit-ClusterVersion-upstream-examples branch from 26eebcb to 9373bf4 Compare August 18, 2021 18:31
@wking
Copy link
Member Author

wking commented Aug 18, 2021

...we happen to find that the 4.6 cluster is still displayed in 4.7 [1] and 4.8 [2] docs...

I was on the fence, but ended up filing a separate #35601 for that. I'm happy to rebase whichever lands second to resolve the conflict.

@shellyyang1989
Copy link

Similar referenced version issue in modules/update-upgrading-cli.adoc:

  1. The referenced output of oc adm upgrade in 4.6 [1], 4.7 [2], 4.8 [3] doc is always cluster 4.1.0. It would be better to get it updated based on the product version respectively.
Cluster version is 4.1.0

Updates:

VERSION IMAGE
4.1.2   quay.io/openshift-release-dev/ocp-release@sha256:9c5f0df8b192a0d7b46cd5f6a4da2289c155fd5302dec7954f8f06c878160b8b
  1. The example output of oc get clusterversion -o json|jq ".items[0].status.history" in in 4.6 [1] doc references 4.1.0, 4.1.2. It would be better to get it updated to 4.5.4, 4.6.3.
[
  {
    "completionTime": null,
    "image": "quay.io/openshift-release-dev/ocp-release@sha256:9c5f0df8b192a0d7b46cd5f6a4da2289c155fd5302dec7954f8f06c878160b8b",
    "startedTime": "2020-11-10T20:30:50Z",
    "state": "Partial",
    "verified": true,
    "version": "4.1.2"
  },
  {
    "completionTime": "2020-11-10T20:30:50Z",
    "image": "quay.io/openshift-release-dev/ocp-release@sha256:b8307ac0f3ec4ac86c3f3b52846425205022da52c16f56ec31cbe428501001d6",
    "startedTime": "2020-11-10T17:38:10Z",
    "state": "Completed",
    "verified": false,
    "version": "4.1.0"
  }
]
  1. The example output of oc get clusterversion -o json|jq ".items[0].status.history" in 4.7 [2] doc references 4.7.0, 4.7.0. It would be better to get it updated to 4.6.9, 4.7.0
[
  {
    "completionTime": null,
    "image": "quay.io/openshift-release-dev/ocp-release@sha256:b8fa13e09d869089fc5957c32b02b7d3792a0b6f36693432acc0409615ab23b7",
    "startedTime": "2021-01-28T20:30:50Z",
    "state": "Partial",
    "verified": true,
    "version": "4.7.0"
  },
  {
    "completionTime": "2021-01-28T20:30:50Z",
    "image": "quay.io/openshift-release-dev/ocp-release@sha256:b8fa13e09d869089fc5957c32b02b7d3792a0b6f36693432acc0409615ab23b7",
    "startedTime": "2021-01-28T17:38:10Z",
    "state": "Completed",
    "verified": false,
    "version": "4.7.0"
  }
]
  1. In 4.8 [3] doc, the example output of oc get clusterversion references a 4.6 cluster. It would be better to reference a 4.7 cluster.
NAME      VERSION   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.6.9     True        False         158m    Cluster version is 4.6.9
  1. In 4.8 [3] doc, the example output of oc get clusterversion -o json|jq ".items[0].status.history" references a 4.8.0, 4.8.0. It would be better to reference 4.7.z, 4.8.0.
[
  {
    "completionTime": null,
    "image": "quay.io/openshift-release-dev/ocp-release@sha256:b8fa13e09d869089fc5957c32b02b7d3792a0b6f36693432acc0409615ab23b7",
    "startedTime": "2021-01-28T20:30:50Z",
    "state": "Partial",
    "verified": true,
    "version": "4.8.0"
  },
  {
    "completionTime": "2021-01-28T20:30:50Z",
    "image": "quay.io/openshift-release-dev/ocp-release@sha256:b8fa13e09d869089fc5957c32b02b7d3792a0b6f36693432acc0409615ab23b7",
    "startedTime": "2021-01-28T17:38:10Z",
    "state": "Completed",
    "verified": false,
    "version": "4.8.0"
  }
]

[1] https://docs.openshift.com/container-platform/4.6/updating/updating-cluster-cli.html#update-upgrading-cli_updating-cluster-cli
[2] https://docs.openshift.com/container-platform/4.7/updating/updating-cluster-cli.html#update-upgrading-cli_updating-cluster-cli
[3] https://docs.openshift.com/container-platform/4.8/updating/updating-cluster-cli.html#update-upgrading-cli_updating-cluster-cli

@shellyyang1989
Copy link

shellyyang1989 commented Aug 19, 2021

Users might want to change the upstream. So it would be better to let them know how to change it by adding a step to describe it in the upgrade doc[1][2].
[1]https://docs.openshift.com/container-platform/4.8/updating/updating-cluster.html#update-upgrading-web_updating-cluster
[2]https://docs.openshift.com/container-platform/4.8/updating/updating-cluster-cli.html#update-upgrading-cli_updating-cluster-cli

@shellyyang1989
Copy link

Above 2 comments are irrelevant to the PR. If you don't want them to be fixed at this point, that's fine and I would be happy to log a bz and ask doc folks to take care of it. Thanks!

@wking
Copy link
Member Author

wking commented Aug 19, 2021

Similar referenced version issue...

Those should all be hashed out in #35601, or possibly a replacement for that PR, because some of the substitutions I attempted weren't working, per screenshots I attached there.

Users might want to change the upstream...

Maybe. That seems orthogonal to me to this PR removing entries from sections of the doc which are not about upstream. Let's punt this point too, so we can land this one and gradually iterate towards where we want to be :)

@shellyyang1989
Copy link

Thanks Trevor.

Similar referenced version issue...

Those should all be hashed out in #35601, or possibly a replacement for that PR, because some of the substitutions I attempted weren't working, per screenshots I attached there.

I'll get the details linked to #35601, so that we can get it fixed in that PR together.

Users might want to change the upstream...

Maybe. That seems orthogonal to me to this PR removing entries from sections of the doc which are not about upstream. Let's punt this point too, so we can land this one and gradually iterate towards where we want to be

Agree. I'll log a bz to track this issue to avoid missing it later on.

Then it looks good to me.

@shellyyang1989
Copy link

LGTM.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 17, 2021
In 4.1, the installer used to explicitly set upstream to our default
URI.  But in openshift/installer#c9095b34518a0
(data/manifests/bootkube/cvo-overrides: Drop the explicit update,
2020-08-28, openshift/installer#4112), which landed in 4.7 and was not
backported, I'd stopped doing that.  In clusters born in 4.7 and
later, the installer will leave upstream unset, and the
cluster-version operator will default to making a reasonable choice.

We still need to talk about explicit upstreams in the case where folks
are pointing their cluster at a local OpenShift Update Service, but
this commit drops the properties where we were incidentally pointing
at the default, Red-Hat-hosted location, because explicitly setting
that value is an anti-pattern that makes it harder for clusters to
adapt if we try to move our default location elsewhere in the future.

Also restore a closing brace and dangling comma to clean up after
c0fc03d (osdocs-2368: updating 4.8 references to 4.9, 2021-10-01, openshift#36974),
which also removed some of the stale 'upstream' references.
@wking wking force-pushed the drop-explicit-ClusterVersion-upstream-examples branch from 9373bf4 to aea8d69 Compare November 17, 2021 20:30
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2021
@wking
Copy link
Member Author

wking commented Nov 17, 2021

Rebased around #36974 with 9373bf434 -> aea8d69. #36974 removed some of the upstream references I'm trying to remove, but also introduced two small JSON typos, which I'm now fixing.

@kalexand-rh kalexand-rh added this to the Next Release milestone Nov 17, 2021
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

LGTM

@kalexand-rh kalexand-rh merged commit fbc8d12 into openshift:main Nov 17, 2021
@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.7

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.8

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.9

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.10

@openshift-cherrypick-robot

@kalexand-rh: #35567 failed to apply on top of branch "enterprise-4.7":

Applying: modules: Drop 'upstream' from ClusterVersion examples
Using index info to reconstruct a base tree...
A	_unused_topics/upgrade-cluster-version-definition.adoc
M	modules/update-upgrading-cli.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/update-upgrading-cli.adoc
CONFLICT (content): Merge conflict in modules/update-upgrading-cli.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 modules: Drop 'upstream' from ClusterVersion examples
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.7

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.

@openshift-cherrypick-robot

@kalexand-rh: #35567 failed to apply on top of branch "enterprise-4.8":

Applying: modules: Drop 'upstream' from ClusterVersion examples
Using index info to reconstruct a base tree...
A	_unused_topics/upgrade-cluster-version-definition.adoc
M	modules/update-upgrading-cli.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/update-upgrading-cli.adoc
CONFLICT (content): Merge conflict in modules/update-upgrading-cli.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 modules: Drop 'upstream' from ClusterVersion examples
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.8

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.

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #38887

Details

In response to this:

/cherrypick enterprise-4.9

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.

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #38888

Details

In response to this:

/cherrypick enterprise-4.10

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.

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

Labels

branch/enterprise-4.7 branch/enterprise-4.8 branch/enterprise-4.9 branch/enterprise-4.10 lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants