Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

chore: add support for Kubernetes v1.21.10 and v1.22.7 on Azure Stack Hub #4846

Merged
merged 10 commits into from
Feb 24, 2022

Conversation

haofan-ms
Copy link
Contributor

@haofan-ms haofan-ms commented Feb 22, 2022

Reason for Change:

Update external-cloud-provider and azuredisk-csi-driver configurations to enable support for K8s 1.21.10 and 1.22.7 on Azure Stack Hub

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

  • No
  • Yes

If "Yes," did you notify that project's maintainers and provide attribution?

  • No
  • Yes

Requirements:

Notes:

@jadarsie
Copy link
Member

jadarsie commented Feb 22, 2022

Please include a brief description of what you are pushing, why we care and how it is accomplished.

@@ -270,7 +270,7 @@ func getComponentDefaultContainerImage(component string, cs *ContainerService) s
k8sComponents := GetK8sComponentsByVersionMap(kubernetesConfig)[cs.Properties.OrchestratorProfile.OrchestratorVersion]
hyperkubeImageBase := kubernetesImageBase
hyperkubeImage := hyperkubeImageBase + k8sComponents[common.Hyperkube]
if cs.Properties.IsAzureStackCloud() {
if cs.Properties.IsAzureStackCloud() && !common.IsKubernetesVersionGe(cs.Properties.OrchestratorProfile.OrchestratorVersion, "1.21.0") {
Copy link
Member

Choose a reason for hiding this comment

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

include please a small comment explaining why the behavior changes starting with 1.21

Copy link
Member

@jadarsie jadarsie Feb 22, 2022

Choose a reason for hiding this comment

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

the alternative is to write code that documents itself

variableWithMeaningfulName := !common.IsKubernetesVersionGe(...)
if cs.Properties.IsAzureStackCloud() && variableWithMeaningfulName { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comment

@@ -1232,6 +1232,9 @@ func (p *Properties) ShouldEnableAzureCloudAddon(addonName string) bool {
if !to.Bool(o.KubernetesConfig.UseCloudControllerManager) {
return false
}
if addonName == common.AzureDiskCSIDriverAddonName && p.IsAzureStackCloud() {
Copy link
Member

Choose a reason for hiding this comment

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

Update the method documentation please:

// For Azure Stack Hub clusters, ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comment

-e AZURE_CORE_ONLY_SHOW_ERRORS="True" \
"${DEV_IMAGE}" make test-kubernetes || exit 1

echo "Installing azuredisk-csi-driver..."
Copy link
Member

Choose a reason for hiding this comment

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

I think this is unnecessary as you can leverage cluster-init in this case: https://github.com/Azure/aks-engine/blob/master/docs/topics/clusterdefinitions.md#components

-e CUSTOM_WINDOWS_PACKAGE_URL=${CUSTOM_WINDOWS_PACKAGE_URL} \
-e AZURE_CORE_ONLY_SHOW_ERRORS="True" \
"${DEV_IMAGE}" make test-kubernetes || tryExit && renameResultsFile "deploy"

Copy link
Member

Choose a reason for hiding this comment

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

I think this can be undone if you use cluster-init

@@ -584,6 +747,15 @@ if [ "${SCALE_CLUSTER}" = "true" ]; then
fi

if [ "${UPGRADE_CLUSTER}" = "true" ]; then
if [ "${DEPLOY_AND_INSTALL_AZUREDISKCSI}" = "true" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to uninstall before upgrade?

@marosset
Copy link
Contributor

Make sure

"c:\akse-cache\win-k8s\" = @(
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.19.15-azs/windowszip/v1.19.15-azs-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.20.11-azs/windowszip/v1.20.11-azs-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.19.16/windowszip/v1.19.16-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.20.15/windowszip/v1.20.15-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.21.10/windowszip/v1.21.10-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.22.7/windowszip/v1.22.7-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.23.4/windowszip/v1.23.4-1int.zip",
"https://kubernetesartifacts.azureedge.net/kubernetes/v1.24.0-alpha.3/windowszip/v1.24.0-alpha.3-1int.zip"
);
gets updated too!

@haofan-ms haofan-ms changed the title chore: add support for Kubernetes v1.21.10 and v1.22.7 on Azure Stack chore: add support for Kubernetes v1.21.10 and v1.22.7 on Azure Stack Hub Feb 24, 2022
Copy link
Contributor

@marosset marosset left a comment

Choose a reason for hiding this comment

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

powershell changes LGTM

@jadarsie jadarsie merged commit 765e5de into Azure:master Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants