release: Support extracting the 'oc' and 'openshift-install' binaries #22439

smarterclayton · 2019-03-30T23:43:44Z

The payload is the authoritative source of tools. To support retrieving the CLI from a payload and also the openshift-install binary being transformed to lock against a particular version (openshift/installer#1422), add support for:

oc adm release extract --command=oc|openshift-install ...

The command attempts to extract the binary appropriate for the current os
or respects --command-os=mac|windows|linux. We do not support alternative
architectures yet but those will be handled by extending the os field if
necessary. We hardcode the lookup locations for now, assuming that a future
release may make this more generic.

On extracting the openshift-install binary we perform the transformation described in openshift/installer#1422

A more general command intended to be used by tooling is

oc adm release extract --tools ... --to=DIR

which creates 5 archives in DIR for each binary+os combo. It also creates a sha256sum.txt.asc file to allow verification.

The 'cli-artifacts' and 'installer-artifacts' images will descend from
'cli' and 'installer' and offer the mac/windows and mac variants of their
respective commands.

This complements but does not replace the "serve CLI tools from console operator via HTTP" story Trevor and Ben were iterating on.

smarterclayton · 2019-03-30T23:46:07Z

/assign @wking

I'm still iterating on this a bit but it "works" (at least, the substitution looks right to me).

pkg/oc/cli/admin/release/extract.go

soltysh · 2019-04-01T11:10:58Z

pkg/oc/cli/admin/release/new.go

-	flags.StringSliceVarP(&o.Filenames, "filename", "f", o.Filenames, "A file defining a mapping of input images to use to build the release")
+	flags.StringSliceVar(&o.Filenames, "filename", o.Filenames, "A file defining a mapping of input images to use to build the release")
 	flags.StringVar(&o.FromImageStream, "from-image-stream", o.FromImageStream, "Look at all tags in the provided image stream and build a release payload from them.")
+	flags.StringVarP(&o.FromImageStreamFile, "from-image-stream-file", "f", o.FromImageStreamFile, "Take the provided image stream on disk and build a release payload from it.")


This will be confusing with all the commands which use -f for --filename. I'd strongly recommend we either don't have a shorthand or have a different one. Also how --from-image-stream-file differs from --from-image-stream is it only for the fact that one points to IS on disk and another to on server, what's the story behind that?

What happens when I specify both?

We don't have any arguments that take both a file and a name on the server. -f is the filename shorthand - you're creating a release from a file. I suppose I could rename --filename to be something else, but this would still probably need to indicate it's a stream.

Renamed --filenames to be --mapping-file but I want to leave --from-image-stream-file to be consistent with the other sources. -f is the right shortcut for that though.

soltysh · 2019-04-01T11:12:41Z

pkg/oc/cli/admin/release/new.go

+				return fmt.Errorf("unable to read input image stream file: %v", err)
+			}
+			is := &imageapi.ImageStream{}
+			if err := yaml.Unmarshal(data, &is); err != nil {


Why doing this manually when you have resource builder?

Release commands don't use resource builder at all. There is no "magic loading", there are only image stream sources.

pkg/oc/cli/admin/release/new.go

pkg/oc/cli/admin/release/extract.go

pkg/oc/cli/admin/release/extract_tools.go

smarterclayton · 2019-04-02T05:01:42Z

This will go in after the rebase so we can start publishing the correct binaries. Will address the current comments, if there are others please look soon.

smarterclayton · 2019-04-02T15:27:16Z

/retest

It should be possible to snapshot a release stream with `oc get is ... -o yaml` and then build a release from it later for archival and historical backup. Support providing an image stream file instead of requiring a live lookup. Refactor the reference loading code to allow spec tags with image IDs to not require a successful import recorded in status, and specifically error when we encounter that condition.

The payload is the authoritative source of tools. To support the openshift-install binary being transformed to lock against a particular version, add support for: oc adm release extract --command=oc|openshift-install ... The command attempts to extract the binary appropriate for the current os or respects --command-os=mac|windows|linux. We do not support alternative architectures yet but those will be handled by extending the os field if necessary. We hardcode the lookup locations for now, assuming that a future release may make this more generic. The 'cli-artifacts' and 'installer-artifacts' images will descend from 'cli' and 'installer' and offer the mac/windows and mac variants of their respective commands. A more general purpose variant of this command is: oc adm release extract --tools ... --to=DIR which creates in DIR an archive for each tool+OS combination (right now, oc win/mac/linux and openshift-install mac/linux) as well as a sha256sum.txt.asc file that can be used to verify the output.

smarterclayton · 2019-04-02T18:15:05Z

updated ptal

smarterclayton · 2019-04-02T20:36:57Z

/retest

smarterclayton · 2019-04-03T00:23:05Z

/retest

smarterclayton · 2019-04-03T14:50:15Z

Talked with @soltysh, will take more feedback after this so I can start integrating with release tooling.

smarterclayton · 2019-04-03T16:00:36Z

/retest

smarterclayton · 2019-04-03T16:09:55Z

/retest

openshift-bot · 2019-04-03T18:04:25Z

/retest

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

soltysh

/lgtm
/retest

openshift-ci-robot · 2019-04-03T19:21:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton, soltysh

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:

~~contrib/completions/OWNERS~~ [smarterclayton,soltysh]
~~pkg/oc/OWNERS~~ [smarterclayton,soltysh]

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

smarterclayton · 2019-04-03T20:02:20Z

/retest

openshift-bot · 2019-04-04T00:17:39Z

/retest

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

smarterclayton · 2019-04-04T01:38:20Z

/retest

smarterclayton · 2019-04-04T02:03:29Z

/retest

smarterclayton · 2019-04-04T02:22:40Z

/retest

openshift-bot · 2019-04-04T03:45:32Z

/retest

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

openshift-bot · 2019-04-04T05:03:30Z

/retest

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

smarterclayton · 2019-04-04T05:25:27Z

/retest

from hells heart I stab at thee unit test flakes

openshift-bot · 2019-04-04T08:05:27Z

/retest

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

soltysh · 2019-04-05T10:44:50Z

from hells heart I stab at thee unit test flakes

You're unit flake attractor, I think you inherited that from @mfojtik 😉 On a more serious note, another flake fix is in #22490 and it looks like HPA has yet another one 🤦‍♂️

…or openshift-install 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).

…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).

This function was a hold-over from the refactor in openshift/origin@a63351b9bb (release: Support extracting the 'oc' and 'openshift-install' binaries, 2019-03-30, openshift/origin#22439) that added extractCommand. Doesn't seem to be worth keeping a separate function for this when we could just call extractCommand directly.

This is a per-extraction-call setting, not a per-extracted-target setting. Targets for a given extraction call are either going to be all archives or never archives. AsArchive landed in openshift/origin@a63351b9bb (release: Support extracting the 'oc' and 'openshift-install' binaries, 2019-03-30, openshift/origin#22439). Dropping the AsZip bit of the loop is fine because we are hard-coding AsZip true for the windows oc in the initial availableTargets declaration.

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 30, 2019

openshift-ci-robot requested review from enj and soltysh March 30, 2019 23:43

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 30, 2019

smarterclayton mentioned this pull request Mar 30, 2019

cmd: Report locked release image pull spec in version command openshift/installer#1505

Merged

smarterclayton changed the title ~~release: Support extracting the 'oc' and 'openshift-install' binaries~~ WIP: release: Support extracting the 'oc' and 'openshift-install' binaries Mar 30, 2019

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 30, 2019

openshift-ci-robot assigned wking Mar 30, 2019

wking reviewed Mar 31, 2019

View reviewed changes

pkg/oc/cli/admin/release/extract.go Outdated Show resolved Hide resolved

smarterclayton force-pushed the release_extract branch from 20f7cd4 to 84c7a83 Compare April 1, 2019 02:20

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 1, 2019

smarterclayton mentioned this pull request Apr 1, 2019

image: Add a production "installer-artifacts" image for Mac binary openshift/installer#1506

Merged

soltysh reviewed Apr 1, 2019

View reviewed changes

wking mentioned this pull request Apr 1, 2019

hack/pin-release-image: Add a helper for pinning release images openshift/installer#1464

Closed

smarterclayton force-pushed the release_extract branch 2 times, most recently from 721621c to 2fac71f Compare April 2, 2019 01:33

smarterclayton changed the title ~~WIP: release: Support extracting the 'oc' and 'openshift-install' binaries~~ release: Support extracting the 'oc' and 'openshift-install' binaries Apr 2, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 2, 2019

smarterclayton force-pushed the release_extract branch from 2fac71f to f1ade98 Compare April 2, 2019 15:04

smarterclayton force-pushed the release_extract branch from f1ade98 to efdfa99 Compare April 2, 2019 18:12

smarterclayton force-pushed the release_extract branch from efdfa99 to a63351b Compare April 2, 2019 18:14

smarterclayton mentioned this pull request Apr 2, 2019

Support --from-image-stream-file on release new for archival purposes #22438

Closed

smarterclayton added the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2019

soltysh approved these changes Apr 3, 2019

View reviewed changes

openshift-ci-robot assigned soltysh Apr 3, 2019

openshift-merge-robot merged commit 4f432d1 into openshift:master Apr 4, 2019

wking mentioned this pull request Apr 4, 2019

what's the best way to automate openshift-install interactions right now? openshift/installer#1533

Closed

wking mentioned this pull request Nov 20, 2019

pkg/cli/admin/release/extract: Drop extractTools openshift/oc#173

Closed

wking mentioned this pull request Nov 20, 2019

pkg/cli/admin/release/extract_tools: Drop AsArchive from extractTarget openshift/oc#175

Closed

release: Support extracting the 'oc' and 'openshift-install' binaries #22439

release: Support extracting the 'oc' and 'openshift-install' binaries #22439

Uh oh!

Conversation

smarterclayton commented Mar 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smarterclayton commented Mar 30, 2019

Uh oh!

Uh oh!

soltysh Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

soltysh Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

smarterclayton Apr 2, 2019

Choose a reason for hiding this comment

Uh oh!

smarterclayton Apr 2, 2019

Choose a reason for hiding this comment

Uh oh!

soltysh Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

smarterclayton Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smarterclayton commented Apr 2, 2019

Uh oh!

smarterclayton commented Apr 2, 2019

Uh oh!

smarterclayton commented Apr 2, 2019

Uh oh!

smarterclayton commented Apr 2, 2019

Uh oh!

smarterclayton commented Apr 3, 2019

Uh oh!

smarterclayton commented Apr 3, 2019

Uh oh!

smarterclayton commented Apr 3, 2019

Uh oh!

smarterclayton commented Apr 3, 2019

Uh oh!

openshift-bot commented Apr 3, 2019

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Apr 3, 2019

Uh oh!

smarterclayton commented Apr 3, 2019

Uh oh!

openshift-bot commented Apr 4, 2019

Uh oh!

smarterclayton commented Apr 4, 2019

Uh oh!

smarterclayton commented Apr 4, 2019

Uh oh!

smarterclayton commented Apr 4, 2019

Uh oh!

openshift-bot commented Apr 4, 2019

Uh oh!

openshift-bot commented Apr 4, 2019

Uh oh!

smarterclayton commented Apr 4, 2019

Uh oh!

openshift-bot commented Apr 4, 2019

Uh oh!

soltysh commented Apr 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

smarterclayton commented Mar 30, 2019 •

edited

Loading

smarterclayton Apr 2, 2019 •

edited

Loading