Skip to content

TELCODOCS-309: BZ1995631 & BZ1999603 / 9/13 Fixed BZs also added to the 4.9 Release Notes#36228

Merged
jeana-redhat merged 1 commit intoopenshift:enterprise-4.9from
ogradyp:TELCODOCS-309
Oct 13, 2021
Merged

TELCODOCS-309: BZ1995631 & BZ1999603 / 9/13 Fixed BZs also added to the 4.9 Release Notes#36228
jeana-redhat merged 1 commit intoopenshift:enterprise-4.9from
ogradyp:TELCODOCS-309

Conversation

@ogradyp
Copy link
Contributor

@ogradyp ogradyp commented Sep 9, 2021

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

netlify bot commented Sep 9, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 10726c6

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/6166fca2b4f7670007197b49

😎 Browse the preview: https://deploy-preview-36228--osdocs.netlify.app/openshift-enterprise/latest/release_notes/ocp-4-9-release-notes

@ogradyp ogradyp changed the title TELCODOCS-309: BZ1995631 added to the 4.9 Release Notes WIP - TELCODOCS-309: BZ1995631 & BZ1999603 added to the 4.9 Release Notes Sep 29, 2021
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 29, 2021
Copy link
Contributor

@ctauchen ctauchen left a comment

Choose a reason for hiding this comment

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

Thanks for this -- a few comments and suggestions for you below.

But first, some housekeeping:

Please remember to add a link to your preview content in your first PR comment. This helps the peer review squad find your builds quickly.

@ctauchen
Copy link
Contributor

@bergerhoffer @bobfuru FYI

Copy link
Contributor

@bobfuru bobfuru left a comment

Choose a reason for hiding this comment

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

Added a couple of comments for consideration. Thanks!

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 6, 2021
@ogradyp ogradyp changed the title WIP - TELCODOCS-309: BZ1995631 & BZ1999603 added to the 4.9 Release Notes TELCODOCS-309: BZ1995631 & BZ1999603 added to the 4.9 Release Notes Oct 6, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 6, 2021
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 8, 2021
@ogradyp ogradyp changed the title TELCODOCS-309: BZ1995631 & BZ1999603 added to the 4.9 Release Notes TELCODOCS-309: BZ1995631 & BZ1999603 / 9/13 Fixed BZs also added to the 4.9 Release Notes Oct 8, 2021
@jeana-redhat jeana-redhat added branch/enterprise-4.9 peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 8, 2021
@jeana-redhat jeana-redhat added this to the Future Release milestone Oct 8, 2021
Copy link
Contributor

@jeana-redhat jeana-redhat left a comment

Choose a reason for hiding this comment

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

In general, there are a handful of terms that I am unsure about. I would expect things like "baremetal-ipi" to be written out (bare metal on installer-provisioned infrastructure?), and things like "ironic-python-agent" to either be enclosed in backticks or written out. But I'm not familiar enough with the bare metal content to know for sure what to recommend - can you check the following against how they are used in the actual content where they are discussed?

  • metal3 (I see both metal3 and Metal3 in the repo)
  • InitContainers/InitContainer
  • baremetal-ipi
  • ironic-python-agent
  • sushy library
  • VirtualMedia.InsertMedia request body
  • BareMetalHost
  • cgroups

Copy link
Contributor

@jeana-redhat jeana-redhat Oct 8, 2021

Choose a reason for hiding this comment

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

Suggested change
* Previously, metal3 pods could not download an RHCOS image due to the sequencing of creating InitContainers. This issue is fixed by reordering the creation of the initContainers, so that InitContainer `metal-static-ip-set` now happens prior to InitContainer `metal3-machine-os-downloader`. The RHCOS image now downloads as expected.
* Previously, metal3 pods could not download an {op-system-first} image due to the sequencing of creating InitContainers. This issue is fixed by reordering the creation of the initContainers, so that InitContainer `metal-static-ip-set` now happens prior to InitContainer `metal3-machine-os-downloader`. The {op-system} image now downloads as expected.

In this item InitContainers is capitalized as InitContainers and initContainers - I don't know which is correct but should probably be the same throughout :) I also see an instance of InitContainer (singular). If this is an API object we would typically say e.g. "InitContainer object" (sing) or "InitContainer objects" (plur) rather than making a plural of the object name. I don't know what they are in this context so please forgive me if this feedback is not relevant/helpful!

Copy link
Contributor Author

@ogradyp ogradyp Oct 11, 2021

Choose a reason for hiding this comment

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

Thanks for your feedback Jeana. Changing occurrences to 'initContainer' as used in the OCP docs https://docs.openshift.com/container-platform/4.8/rest_api/monitoring_apis/alertmanager-monitoring-coreos-com-v1.html . Thanks.

Copy link
Contributor Author

@ogradyp ogradyp Oct 11, 2021

Choose a reason for hiding this comment

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

I have also changed the instances of RHCOS to {op-system-first} / {op-system}. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure about usage of "baremetal-ipi", see main review comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Pardon the drive-by comment here but @jeana-redhat is correct in that we don't use the IPI (or UPI) shorthand in docs and should always spell out. For example, "Previously, when using installer-provisioned installation on bare metal with a host configured...."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Jeana / Bob. I have rewritten as Bob suggests. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure about usage of "ironic-python-agent", see main review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Jeana. This has been updated with backticks. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure about usage of "metal3", see main review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Jeana. 'metal3' is ok. It is already used in the OCP docs https://docs.openshift.com/container-platform/4.8/rest_api/provisioning_apis/provisioning-metal3-io-v1alpha1.html . Thanks.

Comment on lines 1200 to 1342
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Previously, when a cluster with network policies was upgraded from version 4.5 to 4.6, old address sets were orphaned due to a naming convention change in 4.6. This resulted in network policies created in 4.5 with namespace selector criteria not working correctly in 4.6 or later releases. They did not work as they relied on matching old address sets that were not being kept updated with the pod IPs within such namespaces. This issue is fixed with an OVN-Kubernetes upgrade where address sets with old naming conventions are removed, and policies referencing these old address sets are updated, referencing the address sets following the new naming convention.
(link:https://bugzilla.redhat.com/show_bug.cgi?id=1962387[*BZ#1962387*])

This bug showed up under "Networking" in our query and has already been added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct Jeana. Alex pointed me to the 'The Release Notes 4.9 Bug Fix Summaries' excel and I see that 1962387 has already been added. I have removed me version of the text as it's best to align with this excel. I added the text as this BZ also showed up in a query I was using and I wasn't aware of the master excel. Thanks.

Comment on lines 1225 to 1227
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Previously, when pods were created, and then quickly deleted, they were not cleaned up by the kubelet within 20 minutes. This impacted the availability of upgrades, and consequently, the users. This issue is fixed by improving the pod lifecyle logic within the kubelet.
(link:https://bugzilla.redhat.com/show_bug.cgi?id=1952224[*BZ#1952224*])

This bug showed up under "Node" in our query and has already been added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct Jeana. Alex pointed me to the 'The Release Notes 4.9 Bug Fix Summaries' excel and I see that 1952224 has already been added. I have removed me version of the text as it's best to align with this excel. I added the text as this BZ also showed up in a query I was using and I wasn't aware of the master excel. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure about usage of "cgroups", see main review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Jeana. This has been updated with backticks. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The `ip vrf exec` does not work due to a cgroups mismatch. As a result, this command cannot be used inside OpenShift pods. To use virtual routing and forwarding (VRF), applications must be VRF-aware and bind directly to the VRF interface. (link:https://bugzilla.redhat.com/show_bug.cgi?id=1995631[*BZ#1995631*])
* The `ip vrf exec` command does not work due to a cgroups mismatch. As a result, this command cannot be used inside {product-title} pods. To use virtual routing and forwarding (VRF), applications must be VRF-aware and bind directly to the VRF interface. (link:https://bugzilla.redhat.com/show_bug.cgi?id=1995631[*BZ#1995631*])

@jeana-redhat jeana-redhat added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 8, 2021
@ogradyp
Copy link
Contributor Author

ogradyp commented Oct 11, 2021

Thanks for your feedback Jeana. I have addressed you comments in the specific instances. Thanks.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 11, 2021
Copy link
Contributor

@aireilly aireilly left a comment

Choose a reason for hiding this comment

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

A few small comments :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Previously, metal3 pods could not download an RHCOS image due to the sequencing of creating initContainers. This issue is fixed by reordering the creation of the initContainers, so that initContainer `metal-static-ip-set` now happens prior to initContainer `metal3-machine-os-downloader`. The RHCOS image now downloads as expected.
* Previously, metal3 pods could not download an RHCOS image due to the sequencing of creating initContainers. This issue is fixed by reordering the creation of the initContainers, so that the `metal-static-ip-set` initContainer is created before the `metal3-machine-os-downloader` initContainer. The RHCOS image now downloads as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Aidan. I have applied your suggested update. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Previously, when using installer-provisioned installation on bare metal with a host configured to use `idrac-virtualmedia`, the `bios_interface` for that host got set to `idrac-wsman` by default. This resulted in the BIOS settings being unavailable and an exception occurring in the log files. This issue is fixed by using `idrac-redfish` for the default `bios_interface` when using `idrac-virtualmedia`.
* Previously, when using installer-provisioned installation on bare metal with a host configured to use `idrac-virtualmedia`, the `bios_interface` for that host got set to `idrac-wsman` by default. This resulted in the BIOS settings being unavailable and an exception occurring. This issue is fixed by using `idrac-redfish` for the default `bios_interface` when using `idrac-virtualmedia`.

Nit: the exception occurs in the course of application processing, and is recorded in the logs. It doesn't occur in the logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Aidan. I have applied your suggested update. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Previously, following a cluster node reboot, on network configurations specific to OVN-Kubernetes and using link aggregation, cluster network connectivity was lost. This issue is fixed by correctly persisting OVN-Kubernetes specific network configuration across reboots of the cluster nodes.
* Previously, following a cluster node reboot, on network configurations specific to OVN-Kubernetes and using link aggregation, cluster network connectivity was lost. This issue is fixed by correctly persisting the OVN-Kubernetes specific network configuration across reboots of the cluster nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Aidan. I have applied your suggested update. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The `ip vrf exec` does not work due to a `cgroups` mismatch. As a result, this command cannot be used inside OpenShift pods. To use virtual routing and forwarding (VRF), applications must be VRF-aware and bind directly to the VRF interface. (link:https://bugzilla.redhat.com/show_bug.cgi?id=1995631[*BZ#1995631*])
* The `ip vrf exec` command does not work due to a `cgroups` mismatch. As a result, this command cannot be used inside OpenShift pods. To use virtual routing and forwarding (VRF), applications must be VRF-aware and bind directly to the VRF interface. (link:https://bugzilla.redhat.com/show_bug.cgi?id=1995631[*BZ#1995631*])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Aidan. I have applied your suggested update. Thanks.

Copy link
Contributor

@ctauchen ctauchen left a comment

Choose a reason for hiding this comment

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

A few nits, as we're fond of saying.

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a blank line above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Christopher. I have applied your suggested update. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Previously, some error types in the provisioned state caused the host to be deprovisioned. This occurred after a restart of the metal3 pod if the image provisioned to a Bare Metal Host became unavailable. In this case, the host would enter the deprovisioning state. This issue is fixed by modifying the action of the error in the provisioned state so that if the image becomes unavailable, the error will be reported but deprovisioning will not be initiated.
* Previously, some error types in the provisioned state caused the host to be deprovisioned. This occurred after a restart of the metal3 pod if the image provisioned to a bare metal host became unavailable. In this case, the host would enter the deprovisioning state. This issue is fixed by modifying the action of the error in the provisioned state so that if the image becomes unavailable, the error will be reported but deprovisioning will not be initiated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback Christopher. I have applied your suggested update. Thanks.

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 11, 2021
@jeana-redhat jeana-redhat added the lgtm Indicates that a PR is ready to be merged. label Oct 11, 2021
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2021
@openshift-ci
Copy link

openshift-ci bot commented Oct 13, 2021

New changes are detected. LGTM label has been removed.

Copy link
Contributor

@ctauchen ctauchen left a comment

Choose a reason for hiding this comment

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

Just need to clean up the merge conflict helper text. @ogradyp

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 13, 2021
@jeana-redhat jeana-redhat merged commit cfc1f68 into openshift:enterprise-4.9 Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.9 peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants