-
Notifications
You must be signed in to change notification settings - Fork 63
fix: revert oc download from cluster #364
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,20 +3,14 @@ | |
| import os | ||
| import re | ||
| import shlex | ||
| import stat | ||
| import tarfile | ||
| import tempfile | ||
| import zipfile | ||
| from contextlib import contextmanager | ||
| from functools import cache | ||
| from typing import Any, Generator, Optional, Set, Callable | ||
| from json import JSONDecodeError | ||
|
|
||
| import kubernetes | ||
| import platform | ||
| import pytest | ||
| import requests | ||
| from _pytest._py.path import LocalPath | ||
| from _pytest.fixtures import FixtureRequest | ||
| from kubernetes.dynamic import DynamicClient | ||
| from kubernetes.dynamic.exceptions import ( | ||
|
|
@@ -26,7 +20,6 @@ | |
| from ocp_resources.catalog_source import CatalogSource | ||
| from ocp_resources.cluster_service_version import ClusterServiceVersion | ||
| from ocp_resources.config_map import ConfigMap | ||
| from ocp_resources.console_cli_download import ConsoleCLIDownload | ||
| from ocp_resources.data_science_cluster import DataScienceCluster | ||
| from ocp_resources.deployment import Deployment | ||
| from ocp_resources.dsc_initialization import DSCInitialization | ||
|
|
@@ -1153,86 +1146,86 @@ def _get_image_json(cmd: str) -> Any: | |
| raise | ||
|
|
||
|
|
||
| def get_machine_platform() -> str: | ||
| os_machine_type = platform.machine() | ||
| return "amd64" if os_machine_type == "x86_64" else os_machine_type | ||
|
|
||
|
|
||
| def get_os_system() -> str: | ||
| os_system = platform.system().lower() | ||
| if os_system == "darwin" and platform.mac_ver()[0]: | ||
| os_system = "mac" | ||
| return os_system | ||
|
|
||
|
|
||
| def get_oc_console_cli_download_link() -> str: | ||
| oc_console_cli_download = ConsoleCLIDownload(name="oc-cli-downloads", ensure_exists=True) | ||
| os_system = get_os_system() | ||
| machine_platform = get_machine_platform() | ||
| oc_links = oc_console_cli_download.instance.spec.links | ||
| all_links = [ | ||
| link_ref.href | ||
| for link_ref in oc_links | ||
| if link_ref.href.endswith(("oc.tar", "oc.zip")) | ||
| and os_system in link_ref.href | ||
| and machine_platform in link_ref.href | ||
| ] | ||
| LOGGER.info(f"All oc console cli download links: {all_links}") | ||
| if not all_links: | ||
| raise ValueError(f"No oc console cli download link found for {os_system} {machine_platform} in {oc_links}") | ||
|
|
||
| return all_links[0] | ||
|
|
||
|
|
||
| def get_server_cert(tmpdir: LocalPath) -> str: | ||
| data = ConfigMap(name="kube-root-ca.crt", namespace="openshift-apiserver", ensure_exists=True).instance.data[ | ||
| "ca.crt" | ||
| ] | ||
| file_path = os.path.join(tmpdir, "cluster-ca.cert") | ||
| with open(file_path, "w") as fd: | ||
| fd.write(data) | ||
| return file_path | ||
|
|
||
|
|
||
| def download_oc_console_cli(tmpdir: LocalPath) -> str: | ||
| """ | ||
| Download and extract the OpenShift CLI binary. | ||
|
|
||
| Args: | ||
| tmpdir (str): Directory to download and extract the binary to | ||
|
|
||
| Returns: | ||
| str: Path to the extracted binary | ||
|
|
||
| Raises: | ||
| ValueError: If multiple files are found in the archive or if no download link is found | ||
| """ | ||
| oc_console_cli_download_link = get_oc_console_cli_download_link() | ||
| LOGGER.info(f"Downloading archive using: url={oc_console_cli_download_link}") | ||
| cert_file = get_server_cert(tmpdir=tmpdir) | ||
| local_file_name = os.path.join(tmpdir, oc_console_cli_download_link.split("/")[-1]) | ||
| with requests.get(oc_console_cli_download_link, verify=cert_file, stream=True) as created_request: | ||
| created_request.raise_for_status() | ||
| with open(local_file_name, "wb") as file_downloaded: | ||
| for chunk in created_request.iter_content(chunk_size=8192): | ||
| file_downloaded.write(chunk) | ||
| LOGGER.info("Extract the downloaded archive.") | ||
| extracted_filenames = [] | ||
| if oc_console_cli_download_link.endswith(".zip"): | ||
| zip_file = zipfile.ZipFile(file=local_file_name) | ||
| zip_file.extractall(path=tmpdir) | ||
| extracted_filenames = zip_file.namelist() | ||
| else: | ||
| with tarfile.open(name=local_file_name, mode="r") as tar_file: | ||
| tar_file.extractall(path=tmpdir) | ||
| extracted_filenames = tar_file.getnames() | ||
| LOGGER.info(f"Downloaded file: {extracted_filenames}") | ||
|
|
||
| if len(extracted_filenames) > 1: | ||
| raise ValueError(f"Multiple files found in {extracted_filenames}") | ||
| # Remove the downloaded file | ||
| if os.path.isfile(local_file_name): | ||
| os.remove(local_file_name) | ||
| binary_path = os.path.join(tmpdir, extracted_filenames[0]) | ||
| os.chmod(binary_path, stat.S_IRUSR | stat.S_IXUSR) | ||
| return binary_path | ||
| # def get_machine_platform() -> str: | ||
| # os_machine_type = platform.machine() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering if commenting is the best way to stow away changes, I will not block it though.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tox seems to be complaining about unused code https://github.com/opendatahub-io/opendatahub-tests/actions/runs/15758251029/job/44418477232?pr=364 |
||
| # return "amd64" if os_machine_type == "x86_64" else os_machine_type | ||
|
|
||
|
|
||
| # def get_os_system() -> str: | ||
| # os_system = platform.system().lower() | ||
| # if os_system == "darwin" and platform.mac_ver()[0]: | ||
| # os_system = "mac" | ||
| # return os_system | ||
|
|
||
|
|
||
| # def get_oc_console_cli_download_link() -> str: | ||
| # oc_console_cli_download = ConsoleCLIDownload(name="oc-cli-downloads", ensure_exists=True) | ||
| # os_system = get_os_system() | ||
| # machine_platform = get_machine_platform() | ||
| # oc_links = oc_console_cli_download.instance.spec.links | ||
| # all_links = [ | ||
| # link_ref.href | ||
| # for link_ref in oc_links | ||
| # if link_ref.href.endswith(("oc.tar", "oc.zip")) | ||
| # and os_system in link_ref.href | ||
| # and machine_platform in link_ref.href | ||
| # ] | ||
| # LOGGER.info(f"All oc console cli download links: {all_links}") | ||
| # if not all_links: | ||
| # raise ValueError(f"No oc console cli download link found for {os_system} {machine_platform} in {oc_links}") | ||
|
|
||
| # return all_links[0] | ||
|
|
||
|
|
||
| # def get_server_cert(tmpdir: LocalPath) -> str: | ||
| # data = ConfigMap(name="kube-root-ca.crt", namespace="openshift-apiserver", ensure_exists=True).instance.data[ | ||
| # "ca.crt" | ||
| # ] | ||
| # file_path = os.path.join(tmpdir, "cluster-ca.cert") | ||
| # with open(file_path, "w") as fd: | ||
| # fd.write(data) | ||
| # return file_path | ||
|
|
||
|
|
||
| # def download_oc_console_cli(tmpdir: LocalPath) -> str: | ||
| # """ | ||
| # Download and extract the OpenShift CLI binary. | ||
|
|
||
| # Args: | ||
| # tmpdir (str): Directory to download and extract the binary to | ||
|
|
||
| # Returns: | ||
| # str: Path to the extracted binary | ||
|
|
||
| # Raises: | ||
| # ValueError: If multiple files are found in the archive or if no download link is found | ||
| # """ | ||
| # oc_console_cli_download_link = get_oc_console_cli_download_link() | ||
| # LOGGER.info(f"Downloading archive using: url={oc_console_cli_download_link}") | ||
| # cert_file = get_server_cert(tmpdir=tmpdir) | ||
| # local_file_name = os.path.join(tmpdir, oc_console_cli_download_link.split("/")[-1]) | ||
| # with requests.get(oc_console_cli_download_link, verify=cert_file, stream=True) as created_request: | ||
| # created_request.raise_for_status() | ||
| # with open(local_file_name, "wb") as file_downloaded: | ||
| # for chunk in created_request.iter_content(chunk_size=8192): | ||
| # file_downloaded.write(chunk) | ||
| # LOGGER.info("Extract the downloaded archive.") | ||
| # extracted_filenames = [] | ||
| # if oc_console_cli_download_link.endswith(".zip"): | ||
| # zip_file = zipfile.ZipFile(file=local_file_name) | ||
| # zip_file.extractall(path=tmpdir) | ||
| # extracted_filenames = zip_file.namelist() | ||
| # else: | ||
| # with tarfile.open(name=local_file_name, mode="r") as tar_file: | ||
| # tar_file.extractall(path=tmpdir) | ||
| # extracted_filenames = tar_file.getnames() | ||
| # LOGGER.info(f"Downloaded file: {extracted_filenames}") | ||
|
|
||
| # if len(extracted_filenames) > 1: | ||
| # raise ValueError(f"Multiple files found in {extracted_filenames}") | ||
| # # Remove the downloaded file | ||
| # if os.path.isfile(local_file_name): | ||
| # os.remove(local_file_name) | ||
| # binary_path = os.path.join(tmpdir, extracted_filenames[0]) | ||
| # os.chmod(binary_path, stat.S_IRUSR | stat.S_IXUSR) | ||
| # return binary_path | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add verification command for OC CLI installation.
The Rosa CLI installation includes a verification step (
rosa version), but the OC CLI installation lacks similar verification. This could lead to silent failures during the build process.📝 Committable suggestion
🤖 Prompt for AI Agents