Skip to content

CONSOLE-5293: Use console-downloads image instead of cli-artifacts #1158

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.

Sign up for GitHub

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

Open
jhadvig wants to merge 1 commit into
base: main
Choose a base branch
from
Open

CONSOLE-5293: Use console-downloads image instead of cli-artifacts #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bindata/assets/deployments/Untitled
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
readOnlyRootFilesystem
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove this stray file.

bindata/assets/deployments/Untitled is not valid deployment YAML and looks like an accidental editor artifact. Leaving it under the assets tree risks confusing future maintainers or being swept into asset generation by mistake.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/assets/deployments/Untitled` at line 1, Delete the stray asset file
named "Untitled" that contains only the token "readOnlyRootFilesystem" from the
assets/deployments bundle; remove it from the repository and any
asset-generation lists so it won't be packaged into bindata, and if the
readOnlyRootFilesystem setting was intended for a real deployment manifest, move
that key into the proper deployment YAML (not the "Untitled" stub).

201 changes: 4 additions & 197 deletions bindata/assets/deployments/downloads-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,16 @@ spec:
successThreshold: 1
failureThreshold: 3
name: download-server
command:
- /opt/downloads/downloads
args:
- --config-path=/opt/downloads/defaultArtifactsConfig.yaml
Comment on lines +51 to +54
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Local deployment YAML checks =="
rg -n 'readOnlyRootFilesystem|volumeMounts|emptyDir|/tmp' bindata/assets/deployments/downloads-deployment.yaml

echo
echo "== Linked openshift/console downloads server temp-dir checks =="
BASE="https://raw.githubusercontent.com/openshift/console/main"
for p in \
  cmd/downloads/main.go \
  cmd/downloads/config/config.go \
  cmd/downloads/config/artifacts.go
do
  echo "--- ${p} ---"
  curl -fsSL "${BASE}/${p}" 2>/dev/null | rg -n 'TempDir|MkdirTemp|CreateTemp|os\.TempDir|ioutil\.TempDir' || true
done

Repository: openshift/console-operator

Length of output: 451

🏁 Script executed:

#!/bin/bash
# Get full deployment manifest to see all volumes and volumeMounts
cat -n bindata/assets/deployments/downloads-deployment.yaml

Repository: openshift/console-operator

Length of output: 2991

🏁 Script executed:

#!/bin/bash
# Search for any tmp or temp references in the deployment
rg -n 'tmp|temp|TempDir|volumeMounts|volumes' bindata/assets/deployments/downloads-deployment.yaml -i

Repository: openshift/console-operator

Length of output: 85

🏁 Script executed (no clone):

#!/bin/bash
# Check where TempDir is configured in openshift/console
BASE="https://raw.githubusercontent.com/openshift/console/main"
echo "=== cmd/downloads/config/config.go ===" 
curl -fsSL "${BASE}/cmd/downloads/config/config.go" 2>/dev/null | head -100

echo
echo "=== Searching for TempDir assignment in main.go ===" 
curl -fsSL "${BASE}/cmd/downloads/main.go" 2>/dev/null | head -80

Length of output: 1373

🏁 Script executed (no clone):

#!/bin/bash
# Get the full config.go to see how TempDir is initialized
BASE="https://raw.githubusercontent.com/openshift/console/main"
curl -fsSL "${BASE}/cmd/downloads/config/config.go" 2>/dev/null

Length of output: 14

🏁 Script executed:

#!/bin/bash
# Find config files related to downloads
find . -type f -name "*config*" | grep -i download

Repository: openshift/console

Length of output: 104

🏁 Script executed:

#!/bin/bash
# Search for TempDir initialization pattern
rg -n 'TempDir\s*[:=]|ioutil\.TempDir|os\.MkdirTemp' --type go

Repository: openshift/console

Length of output: 2443

🏁 Script executed (no clone):

#!/bin/bash
# Alternative: check if config exists at a different path or structure
curl -fsSL "https://api.github.com/repos/openshift/console/contents/cmd/downloads/config" 2>/dev/null | jq -r '.[] | "\(.name)"' || echo "API check failed"

Length of output: 79

🏁 Script executed:

#!/bin/bash
# Get the full downloads_config.go to see the TempDir initialization
cat -n cmd/downloads/config/downloads_config.go | sed -n '100,140p'

Repository: openshift/console

Length of output: 1431

🏁 Script executed:

#!/bin/bash
# Check git history to see if /tmp volumes existed before
git log --oneline --all -- bindata/assets/deployments/downloads-deployment.yaml | head -5

Repository: openshift/console-operator

Length of output: 460

🏁 Script executed:

#!/bin/bash
# Check if any recent commits removed volume mounts
git log -p -1 -- bindata/assets/deployments/downloads-deployment.yaml 2>/dev/null | head -100

Repository: openshift/console-operator

Length of output: 3133

Add /tmp volumeMount and emptyDir volume — required for runtime temp directory creation.

The new Go downloads server calls os.MkdirTemp("", "artifacts") at startup (verified in openshift/console cmd/downloads/config/downloads_config.go:112), which creates a temporary directory in the system temp location (typically /tmp). With readOnlyRootFilesystem: true and no writable /tmp volume, pod startup will fail.

Previous commits (ea4062a, 29c40d9) added /tmp volumes when enabling read-only root filesystem. This change removed those volumes when switching to the Go server but did not account for the server's temp directory requirement.

Required patch 
       containers:
         - resources:
            requests:
              cpu: 10m
              memory: 50Mi
+          volumeMounts:
+            - name: tmp
+              mountPath: /tmp
           readinessProbe:
       priorityClassName: system-cluster-critical
+      volumes:
+        - name: tmp
+          emptyDir: {}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/assets/deployments/downloads-deployment.yaml` around lines 51 - 54,
The deployment currently runs the downloads binary (command:
/opt/downloads/downloads) with a readOnlyRootFilesystem but lacks a writable
/tmp, causing the Go server's os.MkdirTemp("", "artifacts") to fail; update the
Downloads Deployment to add a volumeMount for /tmp (mountPath: /tmp) in the
container spec and add a corresponding emptyDir volume (name like tmp or
downloads-tmp) in the pod spec so the container has a writable /tmp when
readOnlyRootFilesystem is true.

securityContext:
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
volumeMounts:
- mountPath: /tmp
name: tmp
command:
- /bin/sh
livenessProbe:
httpGet:
path: /
Expand All @@ -75,198 +74,6 @@ spec:
imagePullPolicy: IfNotPresent
terminationMessagePolicy: FallbackToLogsOnError
image: ${IMAGE}
args:
- '-c'
- |
cat <<EOF >/tmp/serve.py
import errno, http.server, os, re, signal, socket, sys, tarfile, tempfile, threading, time, zipfile

def shutdown_handler(signum, frame):
print("Received signal {}, shutting down...".format(signum), flush=True)
os._exit(0)
signal.signal(signal.SIGTERM, shutdown_handler)

def write_index(path, message):
with open(path, 'wb') as f:
f.write('\n'.join([
'<!doctype html>',
'<html lang="en">',
'<head>',
' <meta charset="utf-8">',
'</head>',
'<body>',
' {}'.format(message),
'</body>',
'</html>',
'',
]).encode('utf-8'))

# Launch multiple listeners as threads
class Thread(threading.Thread):
def __init__(self, i, socket):
threading.Thread.__init__(self)
self.i = i
self.socket = socket
self.daemon = True
self.start()

def run(self):
server = http.server.SimpleHTTPRequestHandler
server.server_version = "OpenShift Downloads Server"
server.sys_version = ""
httpd = http.server.HTTPServer(addr, server, False)

# Prevent the HTTP server from re-binding every handler.
# https://stackoverflow.com/questions/46210672/
httpd.socket = self.socket
httpd.server_bind = self.server_close = lambda self: None

httpd.serve_forever()

print('Starting downloads server...', flush=True)
temp_dir = tempfile.mkdtemp()
print('Serving from: {}'.format(temp_dir), flush=True)
os.chdir(temp_dir)

print('Creating arch directories...', flush=True)
for arch in ['amd64', 'arm64', 'ppc64le', 's390x']:
os.mkdir(arch)

content = ['<a href="oc-license">license</a>']
print('Creating license symlink...', flush=True)
os.symlink('/usr/share/openshift/LICENSE', 'oc-license')

# Function to create archives in background
def create_archives_async(arch, operating_system, path, basename, archive_path_root):
try:
print(' [Background] Creating archives for {} {}...'.format(arch, operating_system), flush=True)

print(' [Background] Creating tar archive...', flush=True)
with tarfile.open('{}.tar'.format(archive_path_root), 'w') as tar:
tar.add(path, basename)

print(' [Background] Creating zip archive...', flush=True)
with zipfile.ZipFile('{}.zip'.format(archive_path_root), 'w') as zip:
zip.write(path, basename)

print(' [Background] Done with archives for {} {}'.format(arch, operating_system), flush=True)
except Exception as e:
print(' [Background] ERROR creating archives for {} {}: {}'.format(arch, operating_system, str(e)), flush=True)

print('Creating oc binary symlinks (archives will be created asynchronously)...', flush=True)
archive_threads = []

for arch, operating_system, path in [
('amd64', 'linux', '/usr/share/openshift/linux_amd64/oc'),
('amd64', 'linux', '/usr/share/openshift/linux_amd64/oc.rhel8'),
('amd64', 'linux', '/usr/share/openshift/linux_amd64/oc.rhel9'),
('amd64', 'mac', '/usr/share/openshift/mac/oc'),
('amd64', 'windows', '/usr/share/openshift/windows/oc.exe'),
('arm64', 'linux', '/usr/share/openshift/linux_arm64/oc'),
('arm64', 'linux', '/usr/share/openshift/linux_arm64/oc.rhel8'),
('arm64', 'linux', '/usr/share/openshift/linux_arm64/oc.rhel9'),
('arm64', 'mac', '/usr/share/openshift/mac_arm64/oc'),
('ppc64le', 'linux', '/usr/share/openshift/linux_ppc64le/oc'),
('ppc64le', 'linux', '/usr/share/openshift/linux_ppc64le/oc.rhel8'),
('ppc64le', 'linux', '/usr/share/openshift/linux_ppc64le/oc.rhel9'),
('s390x', 'linux', '/usr/share/openshift/linux_s390x/oc'),
('s390x', 'linux', '/usr/share/openshift/linux_s390x/oc.rhel8'),
('s390x', 'linux', '/usr/share/openshift/linux_s390x/oc.rhel9'),
]:
try:
print(' Processing {} {} ({})...'.format(arch, operating_system, path), flush=True)

# Check if source file exists
if not os.path.exists(path):
print(' WARNING: {} does not exist, skipping'.format(path), flush=True)
continue

file_size = os.path.getsize(path)
print(' Source file size: {} MB'.format(file_size // (1024*1024)), flush=True)

basename = os.path.basename(path)
target_path = os.path.join(arch, operating_system, basename)

print(' Creating directory...', flush=True)
os.makedirs(os.path.join(arch, operating_system), exist_ok=True)

print(' Creating symlink...', flush=True)
os.symlink(path, target_path)

# Only strip .exe extension, keep everything else (e.g., oc.rhel8 stays oc.rhel8)
if basename.endswith('.exe'):
base_root = basename[:-4]
else:
base_root = basename
archive_path_root = os.path.join(arch, operating_system, base_root)

# Start background thread to create archives
archive_thread = threading.Thread(
target=create_archives_async,
args=(arch, operating_system, path, basename, archive_path_root),
daemon=True
)
archive_thread.start()
archive_threads.append(archive_thread)

content.append(
'<a href="{0}">oc ({1} {2})</a> (<a href="{3}.tar">tar</a> <a href="{3}.zip">zip</a>)'.format(
target_path, arch, operating_system, archive_path_root
)
)
print(' Done with {} {} (archives creating in background)'.format(arch, operating_system), flush=True)
except Exception as e:
print(' ERROR processing {} {}: {}'.format(arch, operating_system, str(e)), flush=True)

print('All symlinks created. {} background threads creating archives...'.format(len(archive_threads)), flush=True)

for root, directories, filenames in os.walk(temp_dir):
root_link = os.path.relpath(temp_dir, os.path.join(root, 'child')).replace(os.path.sep, '/')
for directory in directories:
write_index(
path=os.path.join(root, directory, 'index.html'),
message='<p>Directory listings are disabled. See <a href="{}">here</a> for available content.</p>'.format(root_link),
)

write_index(
path=os.path.join(temp_dir, 'index.html'),
message='\n'.join(
['<p><em>Note: Archive files (.tar, .zip) are generated on server startup and may take a few moments to become available.</em></p>'] +
['<ul>'] +
[' <li>{}</li>'.format(entry) for entry in content] +
['</ul>']
),
)

# Create socket
# IPv6 should handle IPv4 passively so long as it is not bound to a
# specific address or set to IPv6_ONLY
# https://stackoverflow.com/questions/25817848/python-3-does-http-server-support-ipv6
try:
addr = ('::', 8080)
sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
except socket.error as err:
# errno.EAFNOSUPPORT is "socket.error: [Errno 97] Address family not supported by protocol"
# When IPv6 is disabled, socket will bind using IPv4.
if err.errno == errno.EAFNOSUPPORT:
addr = ('', 8080)
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
else:
raise
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
print('Binding to {}...'.format(addr), flush=True)
sock.bind(addr)
sock.listen(5)

print('Starting 100 worker threads...', flush=True)
[Thread(i, socket=sock) for i in range(100)]
print('Server ready on port 8080!', flush=True)
time.sleep(9e9)
EOF
exec python3 /tmp/serve.py
volumes:
- name: tmp
emptyDir: {}
tolerations:
- key: node-role.kubernetes.io/master
operator: Exists
Expand Down
2 changes: 1 addition & 1 deletion manifests/07-operator-ibm-cloud-managed.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
- name: CONSOLE_IMAGE
value: registry.svc.ci.openshift.org/openshift:console
- name: DOWNLOADS_IMAGE
value: registry.svc.ci.openshift.org/openshift:cli-artifacts
value: registry.svc.ci.openshift.org/openshift:console-downloads
- name: OPERATOR_IMAGE_VERSION
value: 0.0.1-snapshot
- name: OPERATOR_NAME
Expand Down
2 changes: 1 addition & 1 deletion manifests/07-operator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ spec:
- name: CONSOLE_IMAGE
value: registry.svc.ci.openshift.org/openshift:console
- name: DOWNLOADS_IMAGE
value: registry.svc.ci.openshift.org/openshift:cli-artifacts
value: registry.svc.ci.openshift.org/openshift:console-downloads
- name: OPERATOR_IMAGE_VERSION
value: "0.0.1-snapshot"
- name: OPERATOR_NAME
Expand Down
26 changes: 7 additions & 19 deletions pkg/console/subresource/deployment/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1591,12 +1591,11 @@ func TestWithConsoleNodeSelector(t *testing.T) {
func TestDefaultDownloadsDeployment(t *testing.T) {

var (
defaultReplicaCount int32 = DefaultConsoleReplicas
singleNodeReplicaCount int32 = SingleNodeConsoleReplicas
labels = util.LabelsForDownloads()
gracePeriod int64 = 5
tolerationSeconds int64 = 120
downloadsDeploymentTemplate = resourceread.ReadDeploymentV1OrDie(bindata.MustAsset("assets/deployments/downloads-deployment.yaml"))
defaultReplicaCount int32 = DefaultConsoleReplicas
singleNodeReplicaCount int32 = SingleNodeConsoleReplicas
labels = util.LabelsForDownloads()
gracePeriod int64 = 5
tolerationSeconds int64 = 120
)

type args struct {
Expand Down Expand Up @@ -1680,11 +1679,6 @@ func TestDefaultDownloadsDeployment(t *testing.T) {
Protocol: corev1.ProtocolTCP,
ContainerPort: api.DownloadsPort,
}},
VolumeMounts: []corev1.VolumeMount{{
Name: "tmp",
ReadOnly: false,
MountPath: "/tmp",
}},
ReadinessProbe: &corev1.Probe{
ProbeHandler: corev1.ProbeHandler{
HTTPGet: &corev1.HTTPGetAction{
Expand All @@ -1711,14 +1705,14 @@ func TestDefaultDownloadsDeployment(t *testing.T) {
SuccessThreshold: 1,
FailureThreshold: 3,
},
Command: []string{"/bin/sh"},
Command: []string{"/opt/downloads/downloads"},
Resources: corev1.ResourceRequirements{
Requests: map[corev1.ResourceName]resource.Quantity{
corev1.ResourceCPU: resource.MustParse("10m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
Args: downloadsDeploymentTemplate.Spec.Template.Spec.Containers[0].Args,
Args: []string{"--config-path=/opt/downloads/defaultArtifactsConfig.yaml"},
SecurityContext: &corev1.SecurityContext{
ReadOnlyRootFilesystem: utilpointer.Bool(true),
Capabilities: &corev1.Capabilities{
Expand All @@ -1730,12 +1724,6 @@ func TestDefaultDownloadsDeployment(t *testing.T) {
},
},
},
Volumes: []corev1.Volume{{
Name: "tmp",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
}},
}
downloadsDeploymentPodSpecHighAvail := downloadsDeploymentPodSpecSingleReplica.DeepCopy()
downloadsDeploymentPodSpecHighAvail.Affinity = &corev1.Affinity{
Expand Down