Skip to content

Conversation

@prsurve
Copy link
Contributor

@prsurve prsurve commented Oct 15, 2025

Reason behind the pr:-

When we use the workload before this PR in offline mode, we cannot pull the script from GitHub. so we are building and updating container images which already have script buildin.

WORKDIR arequal
RUN ./autogen.sh ; ./configure ; make ; make install
WORKDIR /
ADD https://raw.githubusercontent.com/red-hat-storage/ocs-workloads/master/rdr/busybox/scripts/simple_io_integrity_check.sh /mnt/simple_io_integrity_check.sh
Copy link
Member

Choose a reason for hiding this comment

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

I don't like an idea of using ADD from the master repository - which will aways require you to merge the change without possibility to test this first. I hope if the context for building of the image is root of the repository we can do something like:

COPY rdr/busybox/scripts/simple_io_integrity_check.sh /mnt/
COPY rdr/busybox/scripts/simple_io.sh /mnt/

Instead of using add from raw repo gitlab URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack will send a commet with this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

RUN ./autogen.sh ; ./configure ; make ; make install
WORKDIR /
ADD https://raw.githubusercontent.com/red-hat-storage/ocs-workloads/master/rdr/busybox/scripts/simple_io_integrity_check.sh /mnt/simple_io_integrity_check.sh
RUN chmod +x /mnt/simple_io_integrity_check.sh
Copy link
Member

Choose a reason for hiding this comment

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

if we do set properly the permission on those files in repository - I think the permission will be carried on via COPY as well inside the image - so if my assumption is correct - you can remove those chmod +x RUN Commands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will check and test it for now will keep as it is

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-balogh, prsurve

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

@prsurve
Copy link
Contributor Author

prsurve commented Nov 10, 2025

/hold

@prsurve
Copy link
Contributor Author

prsurve commented Nov 10, 2025

merging post GA

@prsurve
Copy link
Contributor Author

prsurve commented Nov 18, 2025

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit bc60df0 into red-hat-storage:master Nov 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants