Skip to content
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

Bug 2041940: Fix enablePortPoolsPrepopulation setting definition #42046

Merged

Conversation

MaysaMacedo
Copy link
Contributor

As part of the work to reduce OpenStack resource usage[1]
the Namespace is only now handled when a Pod on Pods
Network is created in it. This new behavior also affects when
the ports pool prepopulation happens requiring update to
the docs.

[1] https://issues.redhat.com/browse/OSASINFRA-2590

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 17, 2022
@MaysaMacedo MaysaMacedo changed the title Fix enablePortPoolsPrepopulation setting definition Bug 2041940: Fix enablePortPoolsPrepopulation setting definition Feb 17, 2022
@openshift-ci openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Feb 17, 2022
@openshift-ci
Copy link

openshift-ci bot commented Feb 17, 2022

@MaysaMacedo: This pull request references Bugzilla bug 2041940, which is invalid:

  • expected the bug to target the "4.11.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 2041940: Fix enablePortPoolsPrepopulation setting definition

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.

@netlify
Copy link

netlify bot commented Feb 17, 2022

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 9277b6c

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/62223d1f95c5860007cca93e

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

@MaysaMacedo
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Feb 17, 2022
@openshift-ci
Copy link

openshift-ci bot commented Feb 17, 2022

@MaysaMacedo: This pull request references Bugzilla bug 2041940, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @eurijon

In response to this:

/bugzilla refresh

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-ci openshift-ci bot requested a review from eurijon February 17, 2022 11:08
@dulek
Copy link
Contributor

dulek commented Feb 17, 2022

Looks good from Kuryr technical side.

@MaysaMacedo MaysaMacedo force-pushed the fix-definition-ports-prepopulation branch from 63b20a0 to abbc022 Compare February 17, 2022 11:48
@gryf
Copy link
Member

gryf commented Feb 17, 2022

Yup, looks good.

@MaysaMacedo
Copy link
Contributor Author

/cc @itzikb-redhat
as you were involved with the epic work.

@MaysaMacedo
Copy link
Contributor Author

/cc @maxwelldb

@maxwelldb
Copy link
Contributor

@MaysaMacedo Which OCP versions does this apply to?

@MaysaMacedo
Copy link
Contributor Author

@MaysaMacedo Which OCP versions does this apply to?

@maxwelldb 4.10+

Copy link
Contributor

@maxwelldb maxwelldb left a comment

Choose a reason for hiding this comment

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

Made some suggestions. @MaysaMacedo

modules/installation-osp-kuryr-port-pools.adoc Outdated Show resolved Hide resolved
modules/installation-osp-kuryr-settings-active.adoc Outdated Show resolved Hide resolved
modules/installation-osp-kuryr-settings-installing.adoc Outdated Show resolved Hide resolved
@maxwelldb
Copy link
Contributor

@MaysaMacedo LMK if you're cool with the suggestions in the comments. If so, we can commit them and then this can go to QE.

@MaysaMacedo
Copy link
Contributor Author

@maxwelldb looks good to me. Thanks!

@maxwelldb maxwelldb added peer-review-needed Signifies that the peer review team needs to review this PR branch/enterprise-4.11 labels Mar 2, 2022
@maxwelldb
Copy link
Contributor

@jboxman Maysa's looking to update content that is in the API docs. I haven't had to do that before. What's the best way to make sure that her changes stick?

@maxwelldb
Copy link
Contributor

@MaysaMacedo It may be that you'll need to make the API docs change outside of the docs repo--probably to: https://github.com/openshift/api/

What looks like API docs on our side are actually generated from there.

@MaysaMacedo MaysaMacedo force-pushed the fix-definition-ports-prepopulation branch from 199b370 to b0f4c20 Compare March 4, 2022 12:18
@MaysaMacedo
Copy link
Contributor Author

@maxwelldb okay, I removed the API docs and proposed the changes against API repo. I have also squashed the commits.

@maxwelldb
Copy link
Contributor

@MaysaMacedo Thanks!

@maxwelldb
Copy link
Contributor

@eurijon Can you +1 this when you have a moment?

@kalexand-rh kalexand-rh 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 Mar 4, 2022
@MaysaMacedo MaysaMacedo force-pushed the fix-definition-ports-prepopulation branch from 6746c6b to 5348fd4 Compare March 4, 2022 15:47
@maxwelldb
Copy link
Contributor

@MaysaMacedo Looks good. If you can just squash this down to one commit, we can merge this after @eurijon +1s it.

As part of the work to reduce OpenStack resource usage[1]
the Namespace is only now handled when a Pod on Pods
Network is created in it. This new behavior also affects when
the ports pool prepopulation happens requiring update to
the docs.

[1] https://issues.redhat.com/browse/OSASINFRA-2590

Co-authored-by: Max Bridges <50179998+maxwelldb@users.noreply.github.com>
Co-authored-by: Kathryn Alexander <37149781+kalexand-rh@users.noreply.github.com>
@MaysaMacedo MaysaMacedo force-pushed the fix-definition-ports-prepopulation branch from 5348fd4 to 9277b6c Compare March 4, 2022 16:23
@MaysaMacedo
Copy link
Contributor Author

@maxwelldb done

@itzikb-redhat
Copy link

LGTM

@MaysaMacedo
Copy link
Contributor Author

@maxwelldb do you think this one is ready?

@eurijon
Copy link

eurijon commented Mar 8, 2022

@maxwelldb it lgtm too

@maxwelldb
Copy link
Contributor

@MaysaMacedo The main and enterprise-4.10 branches are frozen until GA. I can merge this after they're unfrozen, and the change will be reflected in the 4.10 docs that same day.

@bobfuru
Copy link
Contributor

bobfuru commented Mar 10, 2022

In preparation for OCP 4.10 GA, I'm moving the Milestone to "Next Release" as this PR did not make the merge by the 4.10 GA date. Post-GA, any open 4.10 PRs fall under the "Next Release" bucket in the same way that 4.6-4.9 PRs are already there. This change does not have any impact on the work in this PR; it's a housekeeping task to keep account of all PRs that had already merged by the 4.10 GA date. 😁

@bobfuru bobfuru modified the milestones: Future Release, Next Release Mar 10, 2022
@maxwelldb
Copy link
Contributor

@MaysaMacedo Any objections to my merging this?

@MaysaMacedo
Copy link
Contributor Author

@maxwelldb No

@maxwelldb maxwelldb merged commit bac8b4f into openshift:main Mar 11, 2022
@openshift-ci
Copy link

openshift-ci bot commented Mar 11, 2022

@MaysaMacedo: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with /bugzilla refresh.

Bugzilla bug 2041940 has not been moved to the MODIFIED state.

In response to this:

Bug 2041940: Fix enablePortPoolsPrepopulation setting definition

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.

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.10

@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.11

@maxwelldb
Copy link
Contributor

@MaysaMacedo Done. Feel free to adjust your related BZ to reflect this. You should see your changes in prod today.

@openshift-cherrypick-robot

@maxwelldb: new pull request created: #43095

In response to this:

/cherry-pick 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.

@openshift-cherrypick-robot

@maxwelldb: new pull request created: #43096

In response to this:

/cherry-pick enterprise-4.11

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.10 branch/enterprise-4.11 bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. peer-review-done Signifies that the peer review team has reviewed this PR 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.

None yet

9 participants