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

[DPTP-4274] Allow requests for ephemeral-storage in validation/config.go #4444

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Dec 10, 2024

https://issues.redhat.com/browse/DPTP-4274

I have a question in Red Hat Internal #forum-ocp-platform

Can I set ephemeral-storage in the BuildConfig used to build my image?

https://redhat-internal.slack.com/archives/CBN38N3MW/p1722437504182729

The answer given there was a "no". I want it to be yes. I am guessing line 570 in validation/config.go is stopping me. I change line 570.

I did not test this in any way. I don't even have a local checkout of this repo. If you think this is a viable route, I'd test it.

Since there are good prospects getting this in, I added unittest and played with this locally a bit. I also created a Jira ticket.

@openshift-ci openshift-ci bot requested review from psalajova and smg247 December 10, 2024 08:42
Copy link
Contributor

openshift-ci bot commented Dec 10, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jiridanek
Once this PR has been reviewed and has the lgtm label, please assign pruan-rht for approval. For more information see the Kubernetes Code Review Process.

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

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

@jiridanek jiridanek changed the title Allow requests for ephemeral-storage in validation/config.go Allow requests for ephemeral-storage in validation/config.go Dec 10, 2024
@jiridanek
Copy link
Member Author

/label tide/merga-method-squash

Copy link
Contributor

openshift-ci bot commented Dec 10, 2024

@jiridanek: The label(s) /label tide/merga-method-squash cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label tide/merga-method-squash

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.

@jiridanek
Copy link
Member Author

/label tide/merge-method-squash

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 10, 2024
@jiridanek
Copy link
Member Author

error: error creating buildah builder: choosing an image from manifest list docker://image-registry.openshift-image-registry.svc:5000/ci-op-hls38x5i/pipeline@sha256:b318c56ee5d134cbaeba41d1b00143d5a19ba790600be0a7fdfc3df61dc579c0: no image found in manifest list for architecture amd64, variant "", OS linux

did I break the CI?

@jiridanek jiridanek changed the title Allow requests for ephemeral-storage in validation/config.go DPTP-4274: Allow requests for ephemeral-storage in validation/config.go Dec 11, 2024
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 11, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 11, 2024

@jiridanek: This pull request references DPTP-4274 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

I have a question in Red Hat Internal #forum-ocp-platform

Can I set ephemeral-storage in the BuildConfig used to build my image?

https://redhat-internal.slack.com/archives/CBN38N3MW/p1722437504182729

The answer given there was a "no". I want it to be yes. I am guessing line 570 in validation/config.go is stopping me. I change line 570.

I did not test this in any way. I don't even have a local checkout of this repo. If you think this is a viable route, I'd test it.

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 11, 2024

@jiridanek: This pull request references DPTP-4274 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

https://issues.redhat.com/browse/DPTP-4274

I have a question in Red Hat Internal #forum-ocp-platform

Can I set ephemeral-storage in the BuildConfig used to build my image?

https://redhat-internal.slack.com/archives/CBN38N3MW/p1722437504182729

The answer given there was a "no". I want it to be yes. I am guessing line 570 in validation/config.go is stopping me. I change line 570.

I did not test this in any way. I don't even have a local checkout of this repo. If you think this is a viable route, I'd test it.

Since there are good prospects getting this in, I added unittest and played with this locally a bit. I also created a Jira ticket.

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 openshift-eng/jira-lifecycle-plugin repository.

@jiridanek jiridanek changed the title DPTP-4274: Allow requests for ephemeral-storage in validation/config.go [DPTP-4274] Allow requests for ephemeral-storage in validation/config.go Dec 11, 2024
@openshift-ci-robot openshift-ci-robot removed the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 11, 2024
@openshift-ci-robot
Copy link
Contributor

@jiridanek: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

In response to this:

https://issues.redhat.com/browse/DPTP-4274

I have a question in Red Hat Internal #forum-ocp-platform

Can I set ephemeral-storage in the BuildConfig used to build my image?

https://redhat-internal.slack.com/archives/CBN38N3MW/p1722437504182729

The answer given there was a "no". I want it to be yes. I am guessing line 570 in validation/config.go is stopping me. I change line 570.

I did not test this in any way. I don't even have a local checkout of this repo. If you think this is a viable route, I'd test it.

Since there are good prospects getting this in, I added unittest and played with this locally a bit. I also created a Jira ticket.

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link
Contributor

openshift-ci bot commented Dec 11, 2024

@jiridanek: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/images d57b243 link true /test images
ci/prow/security d57b243 link false /test security

Full PR test history. Your PR dashboard.

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.

@smg247
Copy link
Member

smg247 commented Dec 17, 2024

I don't think this will actually do anything. You have modified validation logic which will let your ci-operator config be updated to include ephemeral-storage, but you haven't actually propagated that to the container or pod (or wherever it needs to go).

@jiridanek
Copy link
Member Author

You have modified validation logic which will let your ci-operator config be updated to include ephemeral-storage, but you haven't actually propagated that to the container or pod (or wherever it needs to go).

I did not know something more is still needed. I'm looking for the place that needs to be updating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants