Skip to content

Conversation

abhi44706
Copy link

No description provided.

@abhi44706 abhi44706 marked this pull request as draft August 13, 2025 09:41
@github-actions github-actions bot added the enhancement New feature or request label Aug 13, 2025
Copy link
Member

@thpang thpang left a comment

Choose a reason for hiding this comment

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

Please comment, change, adjust code base given the following comments. Reach out for clarification.

ARG KUBECTL_VERSION=1.32.7

WORKDIR /build
ARG TERRAFORM_VERSION=1.10.5
Copy link
Member

Choose a reason for hiding this comment

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

Is this compatible with the older syntax? Just making sure ;)

&& apt-get install -y python3 python3-dev python3-pip curl unzip gnupg --no-install-recommends \
&& apt-get install -y \
python3 python3-dev python3-pip \
curl unzip gnupg lsb-release ca-certificates software-properties-common \
Copy link
Member

Choose a reason for hiding this comment

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

I understand the extra lines, but wondering about lsb-release, ca-certificates, and software-properties-common

Comment on lines +24 to +45

# Install kubectl
RUN curl -sLO https://dl.k8s.io/release/v${KUBECTL_VERSION}/bin/linux/amd64/kubectl \
&& chmod 755 ./kubectl

# Install helm
RUN curl -fsSL https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 -o get-helm-3 \
&& chmod 755 get-helm-3 \
&& ./get-helm-3 --version v${HELM_VERSION} --no-sudo

# Install terraform (APT + fallback to binary)
RUN set -e \
&& curl -fsSL https://apt.releases.hashicorp.com/gpg | gpg --dearmor -o /usr/share/keyrings/hashicorp.gpg \
&& echo "deb [signed-by=/usr/share/keyrings/hashicorp.gpg] https://apt.releases.hashicorp.com $(lsb_release -cs) main" > /etc/apt/sources.list.d/hashicorp.list \
&& apt-get update || true \
&& (apt-get install -y terraform=${TERRAFORM_VERSION} --no-install-recommends || \
(echo "APT install failed. Falling back to direct download..." && \
curl -fsSL https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip -o terraform.zip \
&& unzip terraform.zip \
&& mv terraform /usr/bin/terraform \
&& chmod +x /usr/bin/terraform \
&& rm terraform.zip)) \
Copy link
Member

Choose a reason for hiding this comment

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

A single RUN command creates a single layer in docker. Not sure why you have broken these items out. Especially since kubectl is one of the lower or base layer. Any change there causes all other RUN commands and build option to rebuild. If that is the intent, then fine, but I am sure you are increasing the size of the docker container.

Comment on lines +35 to +45
RUN set -e \
&& curl -fsSL https://apt.releases.hashicorp.com/gpg | gpg --dearmor -o /usr/share/keyrings/hashicorp.gpg \
&& echo "deb [signed-by=/usr/share/keyrings/hashicorp.gpg] https://apt.releases.hashicorp.com $(lsb_release -cs) main" > /etc/apt/sources.list.d/hashicorp.list \
&& apt-get update || true \
&& (apt-get install -y terraform=${TERRAFORM_VERSION} --no-install-recommends || \
(echo "APT install failed. Falling back to direct download..." && \
curl -fsSL https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip -o terraform.zip \
&& unzip terraform.zip \
&& mv terraform /usr/bin/terraform \
&& chmod +x /usr/bin/terraform \
&& rm terraform.zip)) \
Copy link
Member

Choose a reason for hiding this comment

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

Was this a problem? Never saw where apt install failed? Guessing you all have seen this or this is part of this PR? The title says "Add kubernetes 1.33 support and upgrade Calico to 3.30.0"


VOLUME ["/workspace"]
ENTRYPOINT ["/viya4-iac-k8s/docker-entrypoint.sh"]
ENTRYPOINT ["/viya4-iac-k8s/docker-entrypoint.sh"]
Copy link
Member

Choose a reason for hiding this comment

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

File needs a correct line ending. This can be done in VS Code automatically. This usually happens when switching between windows/linux or your editor, i.e. nano or something strips the last line without adding the correct file ending.

certificatesDir: /etc/kubernetes/pki
kubernetesVersion: v{{ kubernetes_version }}
clusterName: "{{ kubernetes_cluster_name }}"
controlPlaneEndpoint: "{{ kubernetes_vip_fqdn }}:6443"
Copy link
Member

Choose a reason for hiding this comment

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

Is the port static? I have seen where this can change, but not sure if that's a configurable value here.

Comment on lines +47 to +51
{% if kubernetes_version is version('1.31.0', 'lt', version_type='semver') %}
apiVersion: kubelet.config.k8s.io/v1beta1
{% else %}
apiVersion: kubelet.config.k8s.io/v1beta2
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

I don't see v1beta2 for this config. Only v1beta1 and v1alpha1 along with v1. This needs to change. Link - https://kubernetes.io/docs/reference/config-api/kubelet-config.v1/

metricsBindAddress: "0.0.0.0:10249"
enableProfiling: true
clusterCIDR: "{{ kubernetes_pod_subnet }}"
clusterCIDR: "{{ kubernetes_pod_subnet }}"
Copy link
Member

Choose a reason for hiding this comment

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

Fix line ending

Comment on lines +68 to +69
apiVersion: kubeproxy.config.k8s.io/v1beta1
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

There seem to be lots of additions here that are not in the current docs. Please provide info as to where you're finding these apis vs the official doc here - https://kubernetes.io/docs/reference/config-api/kube-proxy-config.v1alpha1/ I believe I am using the latest docs from kubernetes.

ignore_errors: true
if pgrep -x unattended-upgrade >/dev/null; then
killall -q -9 unattended-upgrade
fi
Copy link
Member

Choose a reason for hiding this comment

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

This now fails if the kill does not work? Again, logic change here. Please explain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants