pkg/cli/admin/release/extract: Multi-arch extraction with --filter-by-os

Deprecating the previous --command-os to get consistency with other
image handling.  --filter-by-os originally landed in
openshift/origin@e04b16527b (cli: Mirror images across registries or
to S3, 2017-06-04, openshift/origin#14471).

The wrapping in:

  o.FilterOptions.FilterByOS = fmt.Sprintf("^%s/", o.CommandOperatingSystem)

guards against the unlikely case that a given --command-os value is a
valid prefix for a longer OS, or matches an arch or varient or some
such.

Also teach oc to extract from standard locations e949088 (Enable all
Linux arches in cli-artifacts, 2019-11-07, openshift#153) to avoid choking on
hardlinks [1]:

  $ oc adm release extract --tools registry.svc.ci.openshift.org/ocp/release:4.3.0-0.ci-2019-11-20-121416
  error: image did not contain usr/share/openshift/mac/oc

Teaching extractTarget about architectures avoids conflicts with
multiple architectures trying to use the no-longer-specific-enough
oc-linux in targetsByName [2]:

  $ ./oc adm release extract --command=oc --command-os='*' registry.svc.ci.openshift.org/ocp/release:4.3.0-0.ci-2019-11-20-121416
  error: unable to iterate over layer sha256:13caf755813923e69e25ff1a28cc65766d6dcaa12676e5e274db9ec828e55d71 from registry.svc.ci.openshift.org/ocp/4.3-2019-11-20-121416@sha256:6497d5cb7102903baf98bbc0e07144f04827a21dcd800ff61360d25228a2d2ce: unable to find target with mapping name openshift-client-linux-4.3.0-0.ci-2019-11-20-121416.tar.gz

Adding --command-arch on top of that allows us to set currentArch to
avoid extracting 'oc' for multiple architectures all into the same
file (making it unlikely that you get the architecture you want ;).

Updated the completions with:

  $ make build
  $ hack/update-generated-completions.sh

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1774642
[2]: openshift#172 (comment)

Author: wking

contrib/completions/bash/oc

@@ -5360,6 +5360,9 @@ _oc_adm_release_extract()
     flags+=("--file=")
     two_word_flags+=("--file")
     local_nonpersistent_flags+=("--file=")
+    flags+=("--filter-by-os=")
+    two_word_flags+=("--filter-by-os")
+    local_nonpersistent_flags+=("--filter-by-os=")
     flags+=("--from=")
     two_word_flags+=("--from")
     local_nonpersistent_flags+=("--from=")

contrib/completions/zsh/oc

@@ -5502,6 +5502,9 @@ _oc_adm_release_extract()
     flags+=("--file=")
     two_word_flags+=("--file")
     local_nonpersistent_flags+=("--file=")
+    flags+=("--filter-by-os=")
+    two_word_flags+=("--filter-by-os")
+    local_nonpersistent_flags+=("--filter-by-os=")
     flags+=("--from=")
     two_word_flags+=("--from")
     local_nonpersistent_flags+=("--from=")

pkg/cli/admin/release/extract.go

@@ -2,6 +2,7 @@ package release
 
 import (
 	"archive/tar"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -41,13 +42,13 @@ func NewExtract(f kcmdutil.Factory, parentName string, streams genericclioptions
 			must be installed on the cluster for a given version.
 
 			The --tools and --command flags allow you to extract the appropriate client binaries
-			for	your operating system to disk. --tools will create archive files containing the
-			current OS tools (or, if --command-os is set to '*', all OS versions). Specifying
-			--command for either 'oc' or 'openshift-install' will extract the binaries directly.
-			You may pass a PGP private key file with --signing-key which will create an ASCII
-			armored sha256sum.txt.asc file describing the content that was extracted that is
-			signed by the key. For more advanced signing use the generated sha256sum.txt and an
-			external tool like gpg.
+			for your operating system to disk. --tools will create archive files containing the
+			current OS/arch tools (or, if --filter-by-os is set to '*', all OS/arch versions).
+			Specifying --command for either 'oc' or 'openshift-install' will extract the
+			binaries directly. You may pass a PGP private key file with --signing-key which
+			will create an ASCII armored sha256sum.txt.asc file describing the content that was
+			extracted that is signed by the key. For more advanced signing use the generated
+			sha256sum.txt and an external tool like gpg.
 
 			Instead of extracting the manifests, you can specify --git=DIR to perform a Git
 			checkout of the source code that comprises the release. A warning will be printed
@@ -61,11 +62,13 @@ func NewExtract(f kcmdutil.Factory, parentName string, streams genericclioptions
 			`),
 		Run: func(cmd *cobra.Command, args []string) {
 			kcmdutil.CheckErr(o.Complete(f, cmd, args))
+			kcmdutil.CheckErr(o.Validate())
 			kcmdutil.CheckErr(o.Run())
 		},
 	}
 	flags := cmd.Flags()
 	o.SecurityOptions.Bind(flags)
+	o.FilterOptions.Bind(flags)
 	o.ParallelOptions.Bind(flags)
 
 	flags.StringVar(&o.From, "from", o.From, "Image containing the release payload.")
@@ -77,7 +80,7 @@ func NewExtract(f kcmdutil.Factory, parentName string, streams genericclioptions
 	flags.StringVar(&o.SigningKey, "signing-key", o.SigningKey, "Sign the sha256sum.txt generated by --tools with this GPG key. A sha256sum.txt.asc file signed by this key will be created. The key is assumed to be encrypted.")
 
 	flags.StringVar(&o.Command, "command", o.Command, "Specify 'oc' or 'openshift-install' to extract the client for your operating system.")
-	flags.StringVar(&o.CommandOperatingSystem, "command-os", o.CommandOperatingSystem, "Override which operating system command is extracted (mac, windows, linux). You map specify '*' to extract all tool archives.")
+	flags.StringVar(&o.CommandOperatingSystem, "command-os", o.CommandOperatingSystem, "Deprecated: use --filter-by-os instead.  Override which operating system command is extracted (mac, windows, linux). You map specify '*' to extract all tool archives.")
 	flags.StringVar(&o.FileDir, "dir", o.FileDir, "The directory on disk that file:// images will be copied under.")
 	return cmd
 }
@@ -86,12 +89,14 @@ type ExtractOptions struct {
 	genericclioptions.IOStreams
 
 	SecurityOptions imagemanifest.SecurityOptions
+	FilterOptions   imagemanifest.FilterOptions
 	ParallelOptions imagemanifest.ParallelOptions
 
 	From string
 
 	Tools                  bool
 	Command                string
+	// Deprecated: Use FilterOptions instead.
 	CommandOperatingSystem string
 	SigningKey             string
 
@@ -106,6 +111,27 @@ type ExtractOptions struct {
 }
 
 func (o *ExtractOptions) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string) error {
+	if len(o.CommandOperatingSystem) > 0 {
+		if len(o.FilterOptions.FilterByOS) > 0 {
+			return errors.New("--command-os is deprecated and may not be set when --filter-by-os is set")
+		}
+
+		switch o.CommandOperatingSystem {
+		case "mac":
+			o.CommandOperatingSystem = "darwin"
+		case "*":
+			o.CommandOperatingSystem = ".*"
+		default:
+		}
+
+		o.FilterOptions.FilterByOS = fmt.Sprintf("^%s/", o.CommandOperatingSystem)
+		klog.Warningf("--command-os is deprecated; use --filter-by-os=%q instead", o.FilterOptions.FilterByOS)
+	}
+
+	if err := o.FilterOptions.Complete(cmd.Flags()); err != nil {
+		return err
+	}
+
 	switch {
 	case len(args) == 1 && len(o.From) > 0, len(args) > 1:
 		return fmt.Errorf("you may only specify a single image via --from or argument")
@@ -125,6 +151,10 @@ func (o *ExtractOptions) Complete(f kcmdutil.Factory, cmd *cobra.Command, args [
 	return nil
 }
 
+func (o *ExtractOptions) Validate() error {
+	return o.FilterOptions.Validate()
+}
+
 func (o *ExtractOptions) Run() error {
 	sources := 0
 	if o.Tools {

pkg/cli/admin/release/extract_tools.go

@@ -14,7 +14,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strings"
 	"sync"
@@ -37,9 +36,10 @@ import (
 
 // extractTarget describes how a file in the release image can be extracted to disk.
 type extractTarget struct {
-	OS       string
-	Command  string
-	Optional bool
+	OS           string
+	Architecture string
+	Command      string
+	Optional     bool
 
 	InjectReleaseImage   bool
 	InjectReleaseVersion bool
@@ -138,103 +138,114 @@ var (
 	`)
 )
 
-// extractTools extracts specific commands out of images referenced by the release image.
+// extractCommand extracts specific commands out of images referenced by the release image.
 // TODO: in the future the metadata this command contains might be loaded from the release
 //   image, but we must maintain compatibility with older payloads if so
 func (o *ExtractOptions) extractCommand(command string) error {
 	// Available targets is treated as a GA API and may not be changed without backwards
 	// compatibility of at least N-2 releases.
 	availableTargets := []extractTarget{
 		{
-			OS:      "darwin",
-			Command: "oc",
-			Mapping: extract.Mapping{Image: "cli-artifacts", From: "usr/share/openshift/mac/oc"},
-
-			LinkTo:               []string{"kubectl"},
-			Readme:               readmeCLIUnix,
-			InjectReleaseVersion: true,
-			ArchiveFormat:        "openshift-client-mac-%s.tar.gz",
-		},
-		{
-			OS:      "linux",
-			Command: "oc",
-			Mapping: extract.Mapping{Image: "cli", From: "usr/bin/oc"},
-
-			LinkTo:               []string{"kubectl"},
-			Readme:               readmeCLIUnix,
-			InjectReleaseVersion: true,
-			ArchiveFormat:        "openshift-client-linux-%s.tar.gz",
-		},
-		{
-			OS:      "windows",
-			Command: "oc",
-			Mapping: extract.Mapping{Image: "cli-artifacts", From: "usr/share/openshift/windows/oc.exe"},
-
-			Readme:               readmeCLIWindows,
-			InjectReleaseVersion: true,
-			ArchiveFormat:        "openshift-client-windows-%s.zip",
-			AsZip:                true,
-		},
-		{
-			OS:      "darwin",
-			Command: "openshift-install",
-			Mapping: extract.Mapping{Image: "installer-artifacts", From: "usr/share/openshift/mac/openshift-install"},
+			OS:           "darwin",
+			Architecture: "amd64",
+			Command:      "openshift-install",
+			Mapping:      extract.Mapping{Image: "installer-artifacts", From: "usr/share/openshift/mac/openshift-install"},
 
 			Readme:             readmeInstallUnix,
 			InjectReleaseImage: true,
 			ArchiveFormat:      "openshift-install-mac-%s.tar.gz",
 		},
 		{
-			OS:      "linux",
-			Command: "openshift-install",
-			Mapping: extract.Mapping{Image: "installer", From: "usr/bin/openshift-install"},
+			OS:           "linux",
+			Architecture: "amd64",
+			Command:      "openshift-install",
+			Mapping:      extract.Mapping{Image: "installer", From: "usr/bin/openshift-install"},
 
 			Readme:             readmeInstallUnix,
 			InjectReleaseImage: true,
 			ArchiveFormat:      "openshift-install-linux-%s.tar.gz",
 		},
 		{
-			OS:       "linux",
-			Command:  "openshift-baremetal-install",
-			Optional: true,
-			Mapping:  extract.Mapping{Image: "baremetal-installer", From: "usr/bin/openshift-install"},
+			OS:           "linux",
+			Architecture: "amd64",
+			Command:      "openshift-baremetal-install",
+			Optional:     true,
+			Mapping:      extract.Mapping{Image: "baremetal-installer", From: "usr/bin/openshift-install"},
 
 			Readme:             readmeInstallUnix,
 			InjectReleaseImage: true,
 			ArchiveFormat:      "openshift-baremetal-install-linux-%s.tar.gz",
 		},
 	}
 
-	currentOS := runtime.GOOS
-	if len(o.CommandOperatingSystem) > 0 {
-		currentOS = o.CommandOperatingSystem
-	}
-	if currentOS == "mac" {
-		currentOS = "darwin"
+	for _, arch := range []string{"amd64", "arm64", "ppc64le", "s390x"} {
+		for _, operating_system := range []string{"darwin", "linux", "windows"} {
+			archive_format_os := operating_system
+			basename := "oc"
+			switch operating_system {
+			case "darwin":
+				archive_format_os = "mac"
+				if arch != "amd64" {
+					continue
+				}
+			case "windows":
+				basename += ".exe"
+				if arch != "amd64" {
+					continue
+				}
+			default:
+			}
+
+			archiveFormat := fmt.Sprintf("openshift-client-%s-%s-%%s.tar.gz", archive_format_os, arch)
+			if arch == "amd64" { // backwards compat with single-arch names
+				archiveFormat = fmt.Sprintf("openshift-client-%s-%%s.tar.gz", archive_format_os)
+			}
+
+			source := filepath.Join("usr/share/openshift/", fmt.Sprintf("%s_%s", operating_system, arch), basename)
+			if operating_system == "windows" && arch == "amd64" {
+				// old single-arch name sorts before the new standard name, so the new name is a symlink, and we have to point at the old name
+				source = filepath.Join("usr/share/openshift/", operating_system, basename)
+			}
+
+			availableTargets = append(availableTargets, extractTarget{
+				OS:           operating_system,
+				Architecture: arch,
+				Command:      "oc",
+				Mapping:      extract.Mapping{Image: "cli-artifacts", From: source},
+
+				LinkTo:               []string{"kubectl"},
+				Readme:               readmeCLIUnix,
+				InjectReleaseVersion: true,
+				ArchiveFormat:        archiveFormat,
+			})
+		}
 	}
 
 	// Select the subset of targets based on command line input
 	var willArchive bool
 	var targets []extractTarget
 
-	// Filter by command, or gather all non-optional targets
-	if len(command) > 0 {
-		for _, target := range availableTargets {
-			if target.Command == command {
-				targets = append(targets, target)
-			}
+	commands := sets.NewString()
+	for _, target := range availableTargets {
+		commands.Insert(target.Command)
+		if len(command) > 0 && target.Command != command {
+			continue // filter by command
+		} else if command == "" && target.Optional {
+			continue // no command given, gather only non-optional targets
 		}
-	} else {
-		for _, target := range availableTargets {
-			if !target.Optional {
-				targets = append(targets, target)
-			}
+
+		os_arch := fmt.Sprintf("%s/%s", target.OS, target.Architecture)
+		if !o.FilterOptions.OSFilter.MatchString(os_arch) {
+			klog.V(2).Infof("Skipping %s, %q does not match --filter-by-os=%q", target.ArchiveFormat, os_arch, o.FilterOptions.FilterByOS)
+			continue
 		}
+
+		targets = append(targets, target)
 	}
 
-	// If the user didn't specify a command, or the operating system is set
-	// to '*', we'll produce an archive
-	if len(command) == 0 || o.CommandOperatingSystem == "*" {
+	// If user didn't specify a command, we matched multiple targets,
+	// we'll produce archives
+	if len(command) == 0 || len(targets) > 1 {
 		for i := range targets {
 			targets[i].AsArchive = true
 			targets[i].AsZip = targets[i].OS == "windows"
@@ -243,12 +254,12 @@ func (o *ExtractOptions) extractCommand(command string) error {
 
 	if len(targets) == 0 {
 		switch {
-		case len(command) > 0 && currentOS != "*":
-			return fmt.Errorf("command %q does not support the operating system %q", o.Command, currentOS)
-		case len(command) > 0:
-			return fmt.Errorf("the supported commands are 'oc' and 'openshift-install'")
+		case command == "":
+			return fmt.Errorf("no available commands for --filter-by-os=%q", o.FilterOptions.FilterByOS)
+		case commands.Has(command):
+			return fmt.Errorf("command %s does not support --filter-by-os=%s", command, o.FilterOptions.FilterByOS)
 		default:
-			return fmt.Errorf("no available commands")
+			return fmt.Errorf("the supported commands are: %s", strings.Join(commands.List(), ", "))
 		}
 	}
 
@@ -308,10 +319,6 @@ func (o *ExtractOptions) extractCommand(command string) error {
 	missing := sets.NewString()
 	var validTargets []extractTarget
 	for _, target := range targets {
-		if currentOS != "*" && target.OS != currentOS {
-			klog.V(2).Infof("Skipping %s, does not match current OS %s", target.ArchiveFormat, target.OS)
-			continue
-		}
 		spec, err := findImageSpec(release.References, target.Mapping.Image, o.From)
 		if err != nil {
 			missing.Insert(target.Mapping.Image)
@@ -330,7 +337,7 @@ func (o *ExtractOptions) extractCommand(command string) error {
 			target.Mapping.To = filepath.Join(dir, target.Mapping.Name)
 		} else {
 			target.Mapping.To = filepath.Join(dir, target.Command)
-			target.Mapping.Name = fmt.Sprintf("%s-%s", target.OS, target.Command)
+			target.Mapping.Name = fmt.Sprintf("%s-%s-%s", target.OS, target.Architecture, target.Command)
 		}
 		validTargets = append(validTargets, target)
 	}