Skip to content

Conversation

@lpettyjo
Copy link
Contributor

@lpettyjo lpettyjo commented Mar 24, 2025

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 24, 2025
@lpettyjo lpettyjo added this to the Planned for 4.19 GA milestone Mar 24, 2025
@lpettyjo lpettyjo added the peer-review-needed Signifies that the peer review team needs to review this PR label Mar 24, 2025
@gcharot
Copy link

gcharot commented Mar 24, 2025

LGTM but i would prefer to avoid showing internal namespace references such as clusters-hypershift-ci-339424
Can we replace them with fake names?
thx !

@lahinson lahinson added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Mar 24, 2025
Copy link
Contributor

@lahinson lahinson left a comment

Choose a reason for hiding this comment

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

Looks good! Just one nit and a couple of comments for your consideration.

@lahinson lahinson added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Mar 24, 2025
@gcharot
Copy link

gcharot commented Mar 25, 2025

LGTM thanks!

@ropatil010
Copy link

ropatil010 commented Mar 27, 2025

Should we need to add the execution steps for kubeadmin/normal user privilages which is mentioned in doc: https://docs.google.com/document/d/1QAWt6ZEnUbWJ66-nycyhivNij5Md656nONXdvPhYPKs/edit?disco=AAABgFGCobw
Without those steps it does not work.
cc: @gcharot @tsmetana @dobsonj

@gcharot
Copy link

gcharot commented Mar 27, 2025

is it specific to ROSA ? if so maybe it can be added to the rosa doc and linked from here?

@ropatil010
Copy link

ropatil010 commented Mar 27, 2025

is it specific to ROSA ? if so maybe it can be added to the rosa doc and linked from here?

Hi Gregory,
#90971 (comment)

The execution steps mentioned under limitations 1 and 2 in doc is on all OCP/ROSA clusters.
3rd limitation is specific to rosa-hcp-external-auth profile. Agree we can mention this on ROSA doc.

@dobsonj
Copy link
Member

dobsonj commented Apr 22, 2025

Should we need to add the execution steps for kubeadmin/normal user privilages which is mentioned in doc: https://docs.google.com/document/d/1QAWt6ZEnUbWJ66-nycyhivNij5Md656nONXdvPhYPKs/edit?disco=AAABgFGCobw Without those steps it does not work. cc: @gcharot @tsmetana @dobsonj

Agreed, it would be helpful to include these steps. Setting KUBECONFIG is not enough, you have to be logged in either as admin or as a user with an appropriate clusterrole.

@lpettyjo
Copy link
Contributor Author

lpettyjo commented May 6, 2025

Should we need to add the execution steps for kubeadmin/normal user privilages which is mentioned in doc: https://docs.google.com/document/d/1QAWt6ZEnUbWJ66-nycyhivNij5Md656nONXdvPhYPKs/edit?disco=AAABgFGCobw Without those steps it does not work. cc: @gcharot @tsmetana @dobsonj

Agreed, it would be helpful to include these steps. Setting KUBECONFIG is not enough, you have to be logged in either as admin or as a user with an appropriate clusterrole.

I think I have this documented now. PTAL @dobsonj @tsmetana @gcharot @ropatil010

@openshift-ci
Copy link

openshift-ci bot commented May 6, 2025

@lpettyjo: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

@dobsonj
Copy link
Member

dobsonj commented May 7, 2025

LGTM!

@ropatil010
Copy link

LGTM, Thanks.
The limitation story is resolved. Mentioned the same in doc: https://docs.google.com/document/d/1QAWt6ZEnUbWJ66-nycyhivNij5Md656nONXdvPhYPKs/edit?disco=AAABgFGCobw

Copy link
Member

@tsmetana tsmetana 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 May 9, 2025
@lpettyjo lpettyjo merged commit 9a69615 into openshift:main May 9, 2025
2 checks passed
@lpettyjo
Copy link
Contributor Author

lpettyjo commented May 9, 2025

/cherrypick enterprise-4.19

@openshift-cherrypick-robot

@lpettyjo: new pull request created: #93183

Details

In response to this:

/cherrypick enterprise-4.19

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-sigs/prow repository.

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

Labels

branch/enterprise-4.19 lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants