From 0030b57f12382bbf4a054c3f4f9eef40f8e77d7b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 20 Nov 2019 12:03:59 -0800 Subject: [PATCH] manifests/07-downloads-deployment: Serve from standardized paths Lean on the path consistency from openshift/oc@e949088d (Enable all Linux arches in cli-artifacts, 2019-11-07, openshift/oc#153) to make it easier to scale as we add/remove arch/OS support. With this commit, the downloads deployment is less opinionated about what is in the cli-artifacts bundle. It looks for all possible arch/OS combinations, serves what it finds, and 404s requests for binaries it could not find. I've also renamed mac -> darwin in the canonical URIs, so there's no mental overhead for translating from the conventional GOOS/GOARCH [1]. I've added symlinks in the pod to support external folks who had hard-coded the old URIs, but we should be able to drop those symlinks after a reasonable deprecation period. I have not overhauled the server to 301 requests to the deprecated URI, and we probably want that before dropping the symlinks. [1]: https://golang.org/doc/install/source#environment --- manifests/07-downloads-deployment.yaml | 54 ++++++++++--------- .../controllers/clidownloads/controller.go | 2 +- .../clidownloads/controller_test.go | 6 +-- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/manifests/07-downloads-deployment.yaml b/manifests/07-downloads-deployment.yaml index 1201e176ed..fb746e4bfe 100644 --- a/manifests/07-downloads-deployment.yaml +++ b/manifests/07-downloads-deployment.yaml @@ -59,7 +59,7 @@ spec: - '-c' - | cat <>/tmp/serve.py - import BaseHTTPServer, os, re, signal, SimpleHTTPServer, socket, sys, tarfile, tempfile, threading, time, zipfile + import BaseHTTPServer, errno, os, re, signal, SimpleHTTPServer, socket, sys, tarfile, tempfile, threading, time, zipfile signal.signal(signal.SIGTERM, lambda signum, frame: sys.exit(0)) @@ -85,32 +85,36 @@ spec: temp_dir = tempfile.mkdtemp() print('serving from {}'.format(temp_dir)) os.chdir(temp_dir) - for arch in ['amd64']: + for arch in ['amd64', 'arm64', 'ppc64le', 's390x']: os.mkdir(arch) - for operating_system in ['linux', 'mac', 'windows']: - os.mkdir(os.path.join(arch, operating_system)) - for arch in ['arm64', 'ppc64le', 's390x']: - os.mkdir(arch) - for operating_system in ['linux']: - os.mkdir(os.path.join(arch, operating_system)) + for operating_system in ['linux', 'darwin', 'windows']: + os_path = os.path.join(arch, operating_system) + try: + os.mkdir(os_path) + except OSError as error: + if error.errno != errno.EEXIST: + raise + basename = 'oc' + if operating_system == 'windows': + basename += '.exe' + target_path = os.path.join(os_path, basename) + source_path = os.path.join('/usr', 'share', 'openshift', '{}_{}'.format(operating_system, arch), executable) + try: + os.stat(source_path) + except OSError as error: + print('skipping {}/{}: {}'.format(arch, operating_system, error)) + continue + os.symlink(source_path, target_path) + source_path = os.path.realpath(source_path) + base_root, _ = os.path.splitext(basename) + archive_path_root = os.path.join(arch, operating_system, base_root) + with tarfile.open('{}.tar'.format(archive_path_root), 'w') as tar: + tar.add(source_path, basename) + with zipfile.ZipFile('{}.zip'.format(archive_path_root), 'w') as zip: + zip.write(source_path, basename) - for arch, operating_system, path in [ - ('amd64', 'linux', '/usr/share/openshift/linux_amd64/oc'), - ('amd64', 'mac', '/usr/share/openshift/mac/oc'), - ('amd64', 'windows', '/usr/share/openshift/windows/oc.exe'), - ('arm64', 'linux', '/usr/share/openshift/linux_arm64/oc'), - ('ppc64le', 'linux', '/usr/share/openshift/linux_ppc64le/oc'), - ('s390x', 'linux', '/usr/share/openshift/linux_s390x/oc'), - ]: - basename = os.path.basename(path) - target_path = os.path.join(arch, operating_system, basename) - os.symlink(path, target_path) - base_root, _ = os.path.splitext(basename) - archive_path_root = os.path.join(arch, operating_system, base_root) - with tarfile.open('{}.tar'.format(archive_path_root), 'w') as tar: - tar.add(path, basename) - with zipfile.ZipFile('{}.zip'.format(archive_path_root), 'w') as zip: - zip.write(path, basename) + # backwards compat for old clients. Too bad we can't 301 them... + os.symlink(os.path.join(arch, 'darwin'), os.path.join(arch, 'mac')) # Create socket addr = ('', 8080) diff --git a/pkg/console/controllers/clidownloads/controller.go b/pkg/console/controllers/clidownloads/controller.go index 40155cb264..c2e9c68b0a 100644 --- a/pkg/console/controllers/clidownloads/controller.go +++ b/pkg/console/controllers/clidownloads/controller.go @@ -161,7 +161,7 @@ func PlatformBasedOCConsoleCLIDownloads(host, cliDownloadsName string) *v1.Conso {"Linux for ARM 64", "arm64/linux", "oc.tar"}, {"Linux for IBM Power, little endian", "ppc64le/linux", "oc.tar"}, {"Linux for IBM Z", "s390x/linux", "oc.tar"}, - {"Mac", "amd64/mac", "oc.zip"}, + {"Mac", "amd64/darwin", "oc.zip"}, {"Windows 64-bit", "amd64/windows", "oc.zip"}, } diff --git a/pkg/console/controllers/clidownloads/controller_test.go b/pkg/console/controllers/clidownloads/controller_test.go index f296116bb4..2d76e124ad 100644 --- a/pkg/console/controllers/clidownloads/controller_test.go +++ b/pkg/console/controllers/clidownloads/controller_test.go @@ -59,10 +59,10 @@ func TestGetPlatformURL(t *testing.T) { name: "Test assembling mac specific URL", args: args{ baseURL: "https://www.example.com/amd64", - platform: "mac", + platform: "darwin", archiveType: "oc.zip", }, - want: "https://www.example.com/amd64/mac/oc.zip", + want: "https://www.example.com/amd64/darwin/oc.zip", }, { name: "Test assembling windows 64-bit specific URL", @@ -128,7 +128,7 @@ The oc binary offers the same capabilities as the kubectl binary, but it is furt Text: "Download oc for Linux for IBM Z", }, { - Href: "https://www.example.com/amd64/mac/oc.zip", + Href: "https://www.example.com/amd64/darwin/oc.zip", Text: "Download oc for Mac", }, {