Update for dockerfiles - #1182
Conversation
|
@arpsharm, |
26f02d2 to
a9afcfa
Compare
a9afcfa to
b197ef1
Compare
b197ef1 to
706a69d
Compare
706a69d to
ffe51b7
Compare
ffe51b7 to
fca62aa
Compare
| RUN dnf group install -y "Development Tools" \ | ||
| && dnf install -y podman jq | ||
| ARG python_version=3.14 | ||
| ARG python_full_version=${python_version}.0 |
There was a problem hiding this comment.
Do you always want it to be the .0 version?
| && tar -xzvf /tmp/openshift-client-linux-${OCP_CLIENT_VERSION}.tar.gz -C /tmp/ \ | ||
| && mv /tmp/kubectl /usr/local/bin/kubectl \ | ||
| && mv /tmp/oc /usr/local/bin/oc \ |
There was a problem hiding this comment.
I think kubectl and oc are hard linked; since /tmp and /usr/local/bin are on different filesystems, mv will break the link. You might want to extract them in place, something like:
&& tar xzf /tmp/openshift-client-linux-${OCP_CLIENT_VERSION}.tar.gz -C /tmp kubectl oc
| COPY benchmark_runner/main/main.py /benchmark_runner/main/main.py | ||
|
|
||
| CMD [ "python3.14", "/benchmark_runner/main/main.py"] | ||
| CMD python${PYTHON_VERSION} /benchmark_runner/main/main.py |
There was a problem hiding this comment.
Probably want
CMD [ "python${PYTHON_VERSION}", "/benchmark_runner/main/main.py"]
or some such.
fca62aa to
d5ff3da
Compare
|
Hi @RobertKrawitz Thank you for your review. I've addressed all the changes. Can you please review them now? |
RobertKrawitz
left a comment
There was a problem hiding this comment.
It's necessary to use the most recent point release; I've provided sample code to do it.
| ARG OCP_CLIENT_VERSION=4.19.1 | ||
| ARG VIRTCTL_VERSION=1.5.0 | ||
| ARG python_version=3.14 | ||
| ARG python_patch_version=0 |
There was a problem hiding this comment.
We should always use the latest patch version, not the .0 version, which doesn't have any later bugfixes. That, unfortunately, looks like it's going to require parsing https://www.python.org/ftp/python to determine the latest point release for a given version. Furtunately, the contents of that URL are pretty straightforward to parse.
There was a problem hiding this comment.
Here's some code to find the version:
#!/usr/bin/python3
from html.parser import HTMLParser
import sys
import urllib.request
import urllib.error
url = 'https://www.python.org/ftp/python/'
def FindBestPythonVersion(version):
class FindPythonVersion(HTMLParser):
def __init__(self, release):
super().__init__()
self.release = release
self.start = f'{release}.'
self.best_found_version = None
def handle_starttag(self, tag, attrs):
if tag == 'a':
try:
for name, value in attrs:
if name == 'href':
if value.startswith(self.start):
point = int(value.removeprefix(self.start).rstrip('/'))
if self.best_found_version is None or point > self.best_found_version:
self.best_found_version = point
return
except Exception:
pass
def get_best_version(self):
return self.best_found_version
try:
with urllib.request.urlopen(url) as response:
# Read the content and decode it to UTF-8
data = response.read().decode('utf-8')
except urllib.error.HTTPError as e:
print(f'HTTP error: {e.code} {e.reason}')
sys.exit(1)
except urllib.error.URLError as e:
print(f'URL error: {e.reason}')
sys.exit(1)
parser = FindPythonVersion(version)
try:
parser.feed(data)
if parser.get_best_version() is not None:
print(int(parser.get_best_version()))
except Exception:
print(f"Cannot find python version for {version}",
file=sys.stderr)
FindBestPythonVersion('3.14')
There was a problem hiding this comment.
Hi @RobertKrawitz / @arpsharm ,
I don’t think we should fetch the latest patch version automatically.
Doing so could cause a failure without a clear understanding of the root cause.
We update the version only once it reaches a mature state.
There was a problem hiding this comment.
Patch releases are considered mature (the .0 is probably the least stable). They can contain both security and functionality fixes. See e. g. the 3.14.3 release notes (https://docs.python.org/3/whatsnew/changelog.html#python-3-14-3-final) and similar for 3.14.2 and 3.14.1.
d5ff3da to
7b1edf2
Compare
|
Hey @RobertKrawitz! I've added the script for fetching the latest python patch into hack folder. I've tested it out and it seems to work by getting the latest patch which seems to be Python 3.14.3 as of today. Can you please review all the changes now? |
|
/ok-to-test |
| RUN dnf update -y --nobest \ | ||
| ENV PYTHON_VERSION=${python_version} | ||
|
|
||
| COPY hack/get_latest_python_patch.py /tmp/get_latest_python_patch.py |
There was a problem hiding this comment.
@arpsharm,
We need to put all hard coded including the patch version because the whole dockerfile is not running here.
There was a problem hiding this comment.
What's the specific issue here?
7b1edf2 to
c9809f9
Compare
ebattat
left a comment
There was a problem hiding this comment.
@RobertKrawitz, any more comments ?
Updated prow Dockerfile for newer versions and defined new variables Updated prow Dockerfile for newer versions and defined new variables
c9809f9 to
3012eac
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arpsharm, ebattat, RobertKrawitz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Type of change
Note: Fill x in []
Changes
Both Dockerfiles: CentOS Stream 10, single RUN, ARGs for Python/OCP/virtctl/benchmark-operator/OpenShift4-tools.
Added hack/get_latest_python_patch.py to use latest Python point release at build time; made executable.
prow Dockerfile: OCP 4.21.2, virtctl 1.7.0; main Dockerfile gets these from CI secrets.
Fixed executable bit for ci_pod/jetlag/run_jetlag.sh (pre-commit).