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

pkg/oc/cli/admin/release/extract_tools: Fix false "did not contain" when not hashing #22568

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

wking
Copy link
Member

@wking wking commented Apr 13, 2019

The installer Dockerfile has put the binary in /bin since openshift/install@29e4d10eb7 (openshift/installer#343). Fixes:

$ oc adm release extract --command=openshift-install registry.svc.ci.openshift.org/openshift/origin-release:v4.0
error: image did not contain usr/bin/openshift-install

CC @smarterclayton

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 13, 2019
@wking
Copy link
Member Author

wking commented Apr 13, 2019

Hrm, this is not actually it...

@wking
Copy link
Member Author

wking commented Apr 13, 2019

With master (v4.0.0-alpha.0-2032-gbedcd9e4b6):

oc --loglevel=5 adm release extract --command=openshift-install registry.svc.ci.openshift.org/openshift/origin-release@sha256:f86083d3011a80b8168c507e289f560d30eaa1194521c4571d1674064e607e70 2>&1 | grep 'openshift-install'
hack/build-go.sh cmd/oc 
++ Building go targets for linux/amd64: cmd/oc
[INFO] hack/build-go.sh exited with code 0 after 00h 00m 06s
I0413 09:19:49.573214   28471 extract_tools.go:157] Will extract usr/bin/openshift-install from registry.svc.ci.openshift.org/openshift/origin-v4.0-2019-04-10-154442@sha256:aa5c4f19a216a74e842738431ece16a0e8d4dbae421b7917927e1af586be7a6f
{"architecture":"amd64","config":{"Hostname":"b4be8d00391e","Domainname":"","User":"1000:1000","AttachStdin":false,"AttachStdout":false,"AttachStderr":false,"Tty":false,"OpenStdin":false,"StdinOnce":false,"Env":["PATH=/bin","HOME=/output","OPENSHIFT_BUILD_NAME=installer","OPENSHIFT_BUILD_NAMESPACE=ci-op-8k3mgnkk","container=oci"],"Cmd":null,"ArgsEscaped":true,"Image":"","Volumes":null,"WorkingDir":"/output","Entrypoint":["/bin/openshift-install"],"OnBuild":null,"Labels":{"architecture":"x86_64","authoritative-source-url":"registry.access.redhat.com","build-date":"2019-03-06T02:42:38.249442","com.redhat.build-host":"cpt-0004.osbs.prod.upshift.rdu2.redhat.com","com.redhat.component":"ubi7-container","com.redhat.license_terms":"https://www.redhat.com/licenses/eulas","description":"The Universal Base Image is designed and engineered to be the base layer for all of your containerized applications, middleware and utilities. This base image is freely redistributable, but Red Hat only supports Red Hat technologies through subscriptions for Red Hat products. This image is maintained by Red Hat and updated regularly.","distribution-scope":"public","io.k8s.description":"This is the base image from which all OpenShift images inherit.","io.k8s.display-name":"OpenShift Base","io.openshift.build.commit.author":"","io.openshift.build.commit.date":"","io.openshift.build.commit.id":"3a96753abf479ed2f0fb34fcda0327b0953a571f","io.openshift.build.commit.message":"","io.openshift.build.commit.ref":"master","io.openshift.build.name":"","io.openshift.build.namespace":"","io.openshift.build.source-context-dir":"","io.openshift.build.source-location":"https://github.com/openshift/installer","io.openshift.tags":"base rhel7","name":"ubi7","release":"73","summary":"Provides the latest release of the Red Hat Universal Base Image 7.","url":"https://access.redhat.com/containers/#/registry.access.redhat.com/ubi7/images/7.6-73","vcs-ref":"3a96753abf479ed2f0fb34fcda0327b0953a571f","vcs-type":"git","vcs-url":"https://github.com/openshift/installer","vendor":"Red Hat, Inc.","version":"7.6"}},"container":"faadabfe20561d3e8d9670f8214f0a6a4ab39ae3f34b8e56c9ef26551879282a","container_config":{"Hostname":"installer-build","Domainname":"","User":"","AttachStdin":false,"AttachStdout":false,"AttachStderr":false,"Tty":false,"OpenStdin":false,"StdinOnce":false,"Env":["OPENSHIFT_BUILD_NAME=base","OPENSHIFT_BUILD_NAMESPACE=ci-op-pqq4x736","PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin","container=oci"],"Cmd":["#(imagebuilder)\nsleep 86400"],"Image":"docker-registry.default.svc:5000/ci-op-8k3mgnkk/pipeline@sha256:cbd0e2931e6ae8cbd1522cd7226cff89629c21a3652f6fd57d84f405d432c07a","Volumes":null,"WorkingDir":"","Entrypoint":["/bin/sh","-c"],"OnBuild":null,"Labels":{"architecture":"x86_64","authoritative-source-url":"registry.access.redhat.com","build-date":"2019-03-06T02:42:38.249442","com.redhat.build-host":"cpt-0004.osbs.prod.upshift.rdu2.redhat.com","com.redhat.component":"ubi7-container","com.redhat.license_terms":"https://www.redhat.com/licenses/eulas","description":"The Universal Base Image is designed and engineered to be the base layer for all of your containerized applications, middleware and utilities. This base image is freely redistributable, but Red Hat only supports Red Hat technologies through subscriptions for Red Hat products. This image is maintained by Red Hat and updated regularly.","distribution-scope":"public","io.k8s.description":"This is the base image from which all OpenShift images inherit.","io.k8s.display-name":"OpenShift Base","io.openshift.build.commit.author":"","io.openshift.build.commit.date":"","io.openshift.build.commit.id":"2f60da39a9d2e5cc00293b8ec7ad559fcd32446a","io.openshift.build.commit.message":"","io.openshift.build.commit.ref":"master","io.openshift.build.name":"","io.openshift.build.namespace":"","io.openshift.build.source-context-dir":"base/","io.openshift.build.source-location":"https://github.com/openshift/images","io.openshift.tags":"base rhel7","name":"ubi7","release":"73","summary":"Provides the latest release of the Red Hat Universal Base Image 7.","url":"https://access.redhat.com/containers/#/registry.access.redhat.com/ubi7/images/7.6-73","vcs-ref":"2f60da39a9d2e5cc00293b8ec7ad559fcd32446a","vcs-type":"git","vcs-url":"https://github.com/openshift/images","vendor":"Red Hat, Inc.","version":"7.6"}},"created":"2019-04-10T12:59:40.425201337Z","docker_version":"1.13.1","history":[{"created":"2019-03-06T02:42:55.809549515Z","comment":"Imported from -"},{"created":"2019-03-06T02:43:01.586849Z"},{"created":"2019-04-02T17:28:28.750942629Z","created_by":"#(imagebuilder)\nsleep 86400"},{"created":"2019-04-02T17:29:12.910543215Z","created_by":"#(imagebuilder)\nsleep 86400"},{"created":"2019-04-10T12:59:40.425201337Z","created_by":"#(imagebuilder)\nsleep 86400"}],"os":"linux","rootfs":{"type":"layers","diff_ids":["sha256:4f25b188f48c5192fad4def93d815a8b2e08b7e48b648ef3c4a0cf57cc13ebe2","sha256:e18335b29b2ec6a80d536bf01b4e62cae234b9888b3034e26187695c2b8f2874","sha256:762eded89aa4df31ddeed330bba872be3492b7990ba4a959ac73e5a188b65c35","sha256:4ef6a7eaf5a667bc465b99173030dec8db7baa9e7f4906e5e05ba489702592dc","sha256:c50395eaa2b3569cde887e8163d3bf818abeea18ca92022001dcd79bce8e2a7c"]}}
I0413 09:19:51.887705   28471 extract.go:615] Excluded  due to filter openshift-install
I0413 09:19:51.887765   28471 extract.go:635] Updated name openshift-install
F0413 09:19:56.380917   28471 helpers.go:116] error: image did not contain usr/bin/openshift-install

@wking
Copy link
Member Author

wking commented Apr 13, 2019

$ podman run --rm --entrypoint /bin/sh registry.svc.ci.openshift.org/openshift/origin-v4.0-2019-04-10-154442@sha256:aa5c4f19a216a74e842738431ece16a0e8d4dbae421b7917927e1af586be7a6f
$ ls -l /bin
lrwxrwxrwx. 1 root root 7 Jun 20  2018 /bin -> usr/bin

So usr/bin is the canonical prefix. Still not sure why extract is failing...

@wking
Copy link
Member Author

wking commented Apr 13, 2019

$ podman save --format=oci-dir -o /tmp/installer registry.svc.ci.openshift.org/openshift/origin-v4.0-2019-04-10-154442@sha256:aa5c4f19a216a74e842738431ece16a0e8d4dbae421b7917927e1af586be7a6f
$ jq '.layers[(.layers | length) - 1]' /tmp/installer/manifest.json 
{
  "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
  "digest": "sha256:c50395eaa2b3569cde887e8163d3bf818abeea18ca92022001dcd79bce8e2a7c",
  "size": 151682560
}
$ tar -tf /tmp/installer/c50395eaa2b3569cde887e8163d3bf818abeea18ca92022001dcd79bce8e2a7c 
output/
output/.wh..wh..opq
usr/
usr/bin/
usr/bin/openshift-install

@wking
Copy link
Member Author

wking commented Apr 13, 2019

So extraction seems to successfully create the file. We're just not telling somebody we found it, and are getting a false-alarm warning.

@wking
Copy link
Member Author

wking commented Apr 13, 2019

Ah, because we gate found success on AsArchive now

@wking
Copy link
Member Author

wking commented Apr 13, 2019

So we want something like:

--- a/pkg/oc/cli/admin/release/extract_tools.go
+++ b/pkg/oc/cli/admin/release/extract_tools.go
@@ -311,8 +311,14 @@ func (o *ExtractOptions) extractCommand(command string) error {
                        klog.V(2).Infof("Unable to set extracted file modification time: %v", err)
                }
 
-               // calculate hashes
-               if hash != nil {
+               if hash == nil {
+                       func() {
+                               extractLock.Lock()
+                               defer extractLock.Unlock()
+                               delete(targetsByName, layer.Mapping.Name)
+                       }()
+               } else {
+                       // calculate hashes
                        func() {
                                extractLock.Lock()
                                defer extractLock.Unlock()

but I'm out of time now. Will write it up later.

@wking wking force-pushed the fix-extract-path-for-install branch from 64aa61f to 1c9a3c4 Compare April 13, 2019 21:46
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 13, 2019
…hen not hashing

Stop warning:

  $ oc adm release extract --command=openshift-install registry.svc.ci.openshift.org/openshift/origin-release:v4.0
  error: image did not contain usr/bin/openshift-install

despite successfully extracting the binary.  The bug was from
a63351b (release: Support extracting the 'oc' and
'openshift-install' binaries, 2019-03-30, openshift#22439).
@wking wking changed the title pkg/oc/cli/admin/release/extract_tools: Fix Linux openshift-install path pkg/oc/cli/admin/release/extract_tools: Fix false "did not contain" when not hashing Apr 13, 2019
@wking wking force-pushed the fix-extract-path-for-install branch from 1c9a3c4 to 2d6c76b Compare April 13, 2019 21:46
@wking
Copy link
Member Author

wking commented Apr 13, 2019

Ok, pushed 2d6c76b with a fix for the false warning. As discussed above, the path was fine, and 2d6c76b leaves it alone.

Copy link
Contributor

@markmc markmc 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 - came to the same conclusion in #22636

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmc, soltysh, wking

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 23, 2019
@smarterclayton
Copy link
Contributor

Lgtm as well

@smarterclayton
Copy link
Contributor

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@smarterclayton smarterclayton merged commit 83fcf0f into openshift:master Apr 24, 2019
@wking wking deleted the fix-extract-path-for-install branch April 24, 2019 17:08
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. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants