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

Remove linux-headers apk from Dockerfile (upstream backport to release-4.15) #54

Open
wants to merge 1 commit into
base: release-4.15
Choose a base branch
from

Conversation

thom311
Copy link
Contributor

@thom311 thom311 commented Jun 13, 2024

Backport commit fc002af to release-4.15 branch. This is upstream k8snetworkplumbingwg/ib-sriov-cni#84

Otherwise, build fails:

$ podman build -t foo -f Dockerfile
[1/2] STEP 1/7: FROM golang:1.20-alpine AS builder
[1/2] STEP 2/7: COPY . /usr/src/ib-sriov-cni
--> a78ca05e4982
[1/2] STEP 3/7: ENV HTTP_PROXY $http_proxy
--> a75387849f14
[1/2] STEP 4/7: ENV HTTPS_PROXY $https_proxy
--> 9c3cddc0ab98
[1/2] STEP 5/7: RUN apk add --no-cache --virtual build-dependencies build-base=~0.5 linux-headers=~6.3
fetch https://dl-cdn.alpinelinux.org/alpine/v3.19/main/x86_64/APKINDEX.tar.gz
fetch https://dl-cdn.alpinelinux.org/alpine/v3.19/community/x86_64/APKINDEX.tar.gz
ERROR: unable to select packages:
  linux-headers-6.5-r0:
    breaks: build-dependencies-20240613.095259[linux-headers~6.3]
  build-dependencies-20240613.095259:
    masked in: cache
    satisfies: world[build-dependencies=20240613.095259]
Error: building at STEP "RUN apk add --no-cache --virtual build-dependencies build-base=~0.5 linux-headers=~6.3": while running runtime: exit status 2

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2024
Referring an exact version of the linux-headers package might break image
building process, with errors like:

```
```

Not putting a pinnged version make hadolint complaining:

```
Dockerfile:8 DL3018 warning: Pin versions in apk add. Instead of `apk add <package>` use `apk add <package>=<version>`
```

Signed-off-by: Andrea Panattoni <[email protected]>
Signed-off-by: Thomas Haller <[email protected]>
@openshift-ci openshift-ci bot requested review from dougbtv and s1061123 June 13, 2024 09:55
@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 13, 2024
Copy link

openshift-ci bot commented Jun 13, 2024

Hi @thom311. Thanks for your PR.

I'm waiting for a openshift 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.

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.

@thom311 thom311 changed the base branch from master to release-4.15 June 13, 2024 09:56
@wizhaoredhat
Copy link
Contributor

/approve

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2024
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2024
@SchSeba
Copy link
Contributor

SchSeba commented Jun 23, 2024

/hold

Hi @thom311,
Thanks for the PR but we don't need this for the openshift images as we don't use the u/s Dockerfile
this is the Dockerfile we use https://github.com/openshift/ib-sriov-cni/blob/master/Dockerfile.rhel7 to build d/s images

let me know if we can close this PR or I miss something

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 23, 2024
@thom311
Copy link
Contributor Author

thom311 commented Jun 28, 2024

Dockerfile.rhel7 uses a build image that requires authorization. It was also indicated, that (due to that) it's not permissible to build a container using Dockerfile.rhel7 and publish it for others (e.g. on a public quay repostiroy).

I personally, I can build images using "Dockerfile.rhel7" just fine, because I have an account. The problem is, that I am no conveniently able to share that built, as I would need to prevent unautorized users from getting it.

That means, there is no reasonable way to build an upstream image of ib-sriov-cni. It would require to patch the Dockerfile or write your own.

@SchSeba
Copy link
Contributor

SchSeba commented Jul 14, 2024

/ok-to-test
/lgtm
/approve

@openshift-ci openshift-ci bot 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 Jul 14, 2024
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2024
Copy link

openshift-ci bot commented Jul 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SchSeba, thom311, wizhaoredhat

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

The pull request process is described 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

Copy link

openshift-ci bot commented Jul 14, 2024

@thom311: all tests passed!

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.

@SchSeba
Copy link
Contributor

SchSeba commented Aug 19, 2024

/hold cancel
/label backport-risk-assessed

@openshift-ci openshift-ci bot added backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Aug 19, 2024
@SchSeba
Copy link
Contributor

SchSeba commented Aug 19, 2024

@thom311 you will need to create a Bug for this PR to go in I think

@thom311
Copy link
Contributor Author

thom311 commented Aug 19, 2024

@SchSeba I am reconsidering this.

With sriov-cni, when building a container using the upstream Dockerfile, the build succeeds, but the container doesn't work (in Openshift).

What I really want to do, is being able to build latest master and release-4.x branches, and push them to a public quay.io. I personally can build, since I have credential to access registry.ci.openshift.org. But then I don't think I am allowed to publish the build result.

This makes development and testing cumbersome. And not being able share the build freely is also against open source principles.

What would be a better solution?

@SchSeba
Copy link
Contributor

SchSeba commented Aug 21, 2024

Hi @thom311,
I agree with you. but that is the policy right now from the company :(

so I am fine merging this one here if you want and you can push to your quay (it's private by default) and just add your user or a robot account to your cluster

@thom311
Copy link
Contributor Author

thom311 commented Aug 21, 2024

the problem is that those upstream Dockerfile don't seem to work with the openshift forks. So while this patch fixes an obvious build-issue and makes the build succeed, it does possibly not result in anything useful (at least, it does not do that in general for the openshift/sriov repositories, in particular sriov-cni does not work).

That makes this PR pointless, I will close it. Thanks.

Optimally, we would have Dockerfiles that allow to build the opensource project and get something usable.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. lgtm Indicates that a PR is ready to be merged. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.