Fix kubernetes version for node startup script

[tahsinrahman]

What this PR does / why we need it:
Currently, kubernetes version for node startupscript is taken from machine.Machine.Spec.Versions.ControlPlane, but it should be from machine.Machine.Spec.Versions.Kubelet

Also, if kubernetes version is given with prefix v (e.g v1.13.4), it removes the prefix v

Allow Kubernetes version to be set in generator script

[k8s-ci-robot]

Welcome @tahsinrahman!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-azure 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-azure has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @tahsinrahman. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[awesomenix]

@tahsinrahman Thank you for your first PR. We do have two versions ControlPlane and Kubelet versions, ContolPlane kubernetes version controls controlplane components such as apiserver, scheduler, etc. Kubelet is self explanatory. Kubeadm doesnt like when control plane and kubelet versions are different so the PR makes sense, at same time, we may need this flexibility in future during upgrades.

Can you please redo the PR to pass in both controlplane and kubelet versions and use them accordingly

ControlPlane kubernetes version is used in kubeadm config, while kubelet version will be used during apt-get install.

[awesomenix]

A separate function would be nice here, TrimKubernetesVersion()

[justaugustus]

@tahsinrahman -- Thanks for submitting a PR. :)

A little contrary to @awesomenix's review, I can't see an instance where we'd want to purposely introduce the potential for skew between the control plane components and the kubelet.

I can see the value in making it easier to swap the kubernetes versions, instead of having it hardcoded.

Could you instead:

• Add export KUBERNETES_VERSION="${KUBERNETES_VERSION:-1.13.5}" in the generator script under "Machine settings":

  cluster-api-provider-azure/cmd/clusterctl/examples/azure/generate-yaml.sh

  Line 36 in 149815f

  # Machine settings.

• Replace the hardcoded component versions in machines.yaml.template with the new variable (${KUBERNETES_VERSION}):

  • cluster-api-provider-azure/cmd/clusterctl/examples/azure/machines.yaml.template

    Lines 13 to 14 in 1713947

    	kubelet: 1.13.5
    	controlPlane: 1.13.5

  • cluster-api-provider-azure/cmd/clusterctl/examples/azure/machines.yaml.template

    Line 44 in 1713947

    kubelet: 1.13.5

[awesomenix]

I can't see an instance where we'd want to purposely introduce the potential for skew between the control plane components and the kubelet.

During Upgrades, in this guide https://kubernetes.io/docs/tasks/administer-cluster/kubeadm/kubeadm-upgrade-1-13/

1. Upgrade kubeadm
2. Upgrade control plane version
3. Upgrade kubectl
4. Upgrade kubelet

If above workflow needs to be automated, then you need controlplane and kubelet versions to be passed into the vm startup scripts or upgrade scripts. This may not be the only scenario, customers may want separate machinesets to have different versions for testing, but that would be a kubelet version.

Having separate versions for controlplane and kubelet is useful, but has no meaning during bootstrap process, since kubeadm doesnt like the skew

[justaugustus]

Right. In the context of this project, we're only handling cluster reconciliation and machine instantiation i.e., once the bootstrap process for an Azure virtual machine happens, it doesn't happen again.

Which means, for our case, skew between the components is not desired.
Bootstrapping an initial kubeadm cluster is in-scope. Upgrading kubeadm is not.
Adding MachineSets would currently be a post-bootstrap task, so the user would have to create the manifests for them and set the versions themselves anyway.

If we can think of alternative use cases (outside of upgrade) where version skew might be appropriate, I could be open to exploring, but overall, it doesn't seem like a good direction to move in.

[tahsinrahman]

I've updated the pr, can you check? @justaugustus @awesomenix

[justaugustus]

Thanks for updating this, @tahsinrahman!
/ok-to-test
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justaugustus, tahsinrahman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [justaugustus]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[justaugustus]

@tahsinrahman -- can you make sure your change is gofmt-ed?
Currently getting this error in the test suite:

pkg/cloud/azure/services/config/startupscript.go:120: File is not `gofmt`-ed with `-s` (gofmt)
KubernetesVersion: trimKubernetesVersion(machine.Machine.Spec.Versions.Kubelet),

[justaugustus]

/lgtm