Skip to content

Add GROUP variable for chown where different from USER #1155

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Nov 20, 2020
Merged

Add GROUP variable for chown where different from USER #1155

openshift-merge-robot merged 1 commit into from
Nov 20, 2020

Conversation

@mrhillsman
Copy link
Contributor

@mrhillsman mrhillsman commented Nov 19, 2020 •
edited
Loading

whoami does not always match the group name for
the user. Use id -gn to set the group name properly.
Also create GROUP environment variable and replace
USER where appropriate for chown commands.

Fix #1154

Signed-off-by: Melvin Hillsman mrhillsman@redhat.com

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 19, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 19, 2020

Hi @mrhillsman. Thanks for your PR.

I'm waiting for a openshift-metal3 member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

@mrhillsman mrhillsman changed the title Fixes openshift-metal3/dev-scripts#1154 Fixes #1154 Nov 19, 2020
@mrhillsman mrhillsman changed the title Fixes #1154 Fix openshift-metal3/dev-scripts#1154 Nov 19, 2020
@dhellmann
Copy link
Member

dhellmann commented Nov 19, 2020

Hi, @mrhillsman!

/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 19, 2020
@mrhillsman mrhillsman changed the title Fix openshift-metal3/dev-scripts#1154 Add GROUP variable for runs of chown Nov 19, 2020
@mrhillsman mrhillsman changed the title Add GROUP variable for runs of chown Add GROUP variable for chown where different from USER Nov 19, 2020
@hardys
Copy link

hardys commented Nov 19, 2020

Thanks for the fix @mrhillsman! LGTM except please can you fix the commit message, it should be something like:

    Fix incorrect group name

     `whoami` does not always match the group name for
    the user. Use `id -gn` to set the group name properly.
    Also create GROUP environment variable and replace
    USER where appropriate for chown commands.

@mrhillsman
Fix incorrect group name
469ce0b 
`whoami` does not always match the group name for
the user. Use `id -gn` to set the group name properly.
Also create GROUP environment variable and replace
USER where appropriate for chown commands.

Signed-off-by: Melvin Hillsman <mrhillsman@redhat.com>
@mrhillsman
Copy link
Contributor Author

mrhillsman commented Nov 19, 2020

Thanks for the fix @mrhillsman! LGTM except please can you fix the commit message, it should be something like:

    Fix incorrect group name

     `whoami` does not always match the group name for
    the user. Use `id -gn` to set the group name properly.
    Also create GROUP environment variable and replace
    USER where appropriate for chown commands.

Done

@openshift-merge-robot
Copy link

openshift-merge-robot commented Nov 19, 2020

@mrhillsman: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-metal-ipi-ovn-dualstack 469ce0b link /test e2e-metal-ipi-ovn-dualstack

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

@hardys
Copy link

hardys commented Nov 20, 2020

/approve

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 20, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 20, 2020
@stbenjam
Copy link
Member

stbenjam commented Nov 20, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2020
@openshift-merge-robot openshift-merge-robot merged commit c278194 into openshift-metal3:master Nov 20, 2020
Avielyo10 pushed a commit to Avielyo10/dev-scripts that referenced this pull request Dec 2, 2020
Additional trust bundle - fix
3c41002 
* Simplify additional_trust_bundle()
* Pass ca via path instead of via env vars

Improve readability

Fix loca-bmo script (openshift-metal3#1144) (openshift-metal3#1084)

Co-authored-by: Andrea Fasano <60063538+andfasano@users.noreply.github.com>

Fix incorrect group name (openshift-metal3#1155)

`whoami` does not always match the group name for
the user. Use `id -gn` to set the group name properly.
Also create GROUP environment variable and replace
USER where appropriate for chown commands.

Signed-off-by: Melvin Hillsman <mrhillsman@redhat.com>

Additional trust bundle (openshift-metal3#1152)

* Adding the option to add certs to install-config.yaml

* Add ADDITIONAL_TRUST_BUNDLE to the config_example.sh

* Avoid additionalTrustBundle duplication & move from sed to awk
When MIRROR_IMAGES is set or when using ipv6 (this is always true for ipv6)
additionalTrustBundle section gets created, so give precedence to MIRROR_IMAGES CA

* In case MIRROR_IMAGES is set, concat ADDITIONAL_TRUST_BUNDLE

Co-authored-by: Aviel Yosef <ayosef@redhat.com>

Account for podman version < 2.0.0 (openshift-metal3#1151)

The default for CentOS 8 is 1.6.4 which does not support units
in `podman wait -i`. Add a version check to support both.

Follow-up to commit 42af610.

Update version detection and make dnsVIP conditional (openshift-metal3#1146)

* Store local copy of release info

We retrive this several times, so caching locally will be
faster, and also provides a useful reference for debugging

* Replace OPENSHIFT_VERSION reference with function

This variable isn't always set, in particular in CI, so instead
of relying on it, we can use a new openshift_version function
which derives the version from the locally cached release info.

* Only specify dnsVIP for < 4.5

This was removed in openshift/installer#3304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cybertron cybertron Awaiting requested review from cybertron

@hardys hardys Awaiting requested review from hardys

Assignees

@stbenjam stbenjam

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error when chown runs and user name differs from group name

6 participants

@mrhillsman @openshift-ci-robot @dhellmann @hardys @openshift-merge-robot @stbenjam