[CAFV-267] Clutser upgrade test automation #475

ymo24 · 2023-07-10T23:54:50Z

Description

Please provide a brief description of the changes proposed in this Pull Request

Checklist

tested locally
updated any relevant dependencies
updated any relevant documentation or examples

API Changes

Are there API changes?

Yes
No

If yes, please fill in the below

Updated conversions?
- Yes
- No
- N/A
Updated CRDs?
- Yes
- No
- N/A
Updated infrastructure-components.yaml?
- Yes
- No
- N/A
Updated ./examples/capi-quickstart.yaml?
- Yes
- No
- N/A
Updated necessary files under ./infrastructure-vcd/v1.0.0/?
- Yes
- No
- N/A

Issue

If applicable, please reference the relevant issue

Fixes #

This change is

Signed-off-by: ymo24 <[email protected]>

rliang88

Reviewed 2 of 7 files at r1, all commit messages.
Reviewable status: 2 of 7 files reviewed, all discussions resolved (waiting on @Anirudh9794 and @sahithi)

Anirudh9794

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @sahithi)

Anirudh9794

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sahithi and @ymo24)

tests/e2e/utils/cluster_upgrade_utils.go line 375 at r1 (raw file):

			break
		case VCDMachineTemplate:
			specMap, err := testingsdk.GetMapBySpecName(unstructuredObj.Object, "spec", "VCDMachineTemplate")

We should ideally create a new VCDMachineTemplate object and change the reference in the Machine Deployment and KCP.
But I am okay with the current changes.
I don't know if we are planning to use VCDMachineTemplate this way. Can you please add a TODO for this ?

sahithi

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @Anirudh9794 and @ymo24)

tests/e2e/e2e_suite_test.go line 13 at r1 (raw file):

	PathToMngmntClusterKubecfg    string
	PathToWorkloadClusterCapiYaml string
	TargetTKGVersion              string

It is not clear if these variables TargetTKGVersion and TargetK8SVersion are meant to be used for cluster creation (or) cluster upgrades.

Please rename them to TargetTKGVersionForUpgradeTest, TargetK8sVersionForUpgradeTest

tests/e2e/workload_cluster_upgrade_test.go line 32 at r1 (raw file):

var _ = Describe("Workload Cluster Life cycle management", func() {
	When("create, upgrade, and delete the workload cluster", func() {

For my benefit, what is the purpose of this "When"?

tests/e2e/utils/cluster_upgrade_utils.go line 68 at r1 (raw file):

// @param currTkgVersion - the cluster's current tkg version
// @param tkgMap - tkg_map.json, but marshalled into map[string]interface{}
// @return map[string]interface{} - map of all possible upgrade paths; it is a subset of tkgMap

Can you mention an example of return map? It is a good practice to identify key-value pairs by an example.

tests/e2e/utils/cluster_upgrade_utils.go line 150 at r1 (raw file):

		}

		tkgVersionIsGreater, err := isGreaterThanOrEqual(tkgVersionStr, currTkgVersion)

you can rename this to isTkgVersionCompatible.

TKG version need not be greater.

tests/e2e/utils/cluster_upgrade_utils.go line 177 at r1 (raw file):

func getRDEUpgradeResources(supportedUpgrades map[string]interface{}, allVappTemplates []*types.QueryResultVappTemplateType, targetK8sVersion, targetTkgVersion string) (string, *types.QueryResultVappTemplateType, error) {
	// Create a map to store the tkgOva data based on the k8s version
	tkgOvaMap := make(map[string]map[string]interface{})

Please provide examples on how these maps will look like.
for example, what is the key - tkg version? K8s version? ova name? and what is the value?

I had to read through the entire method and then tkgmap.json to find that out.

tests/e2e/utils/cluster_upgrade_utils.go line 252 at r1 (raw file):

// - string: The Kubernetes version that the cluster is being upgraded to.
// - error: nil if there is no error, otherwise the encountered error.
func getCapiYamlAfterUpgrade(ctx context.Context, r runtimeclient.Client, capiYaml string, tkgMapKey string, tkgMap map[string]interface{}, vappTemplate *types.QueryResultVappTemplateType) (string, error) {

Method name and return value do not match.

Please change it to ApplyK8sVersionUpgrade()

tests/e2e/utils/cluster_upgrade_utils.go line 375 at r1 (raw file):

Previously, Anirudh9794 wrote…

We should ideally create a new VCDMachineTemplate object and change the reference in the Machine Deployment and KCP.
But I am okay with the current changes.
I don't know if we are planning to use VCDMachineTemplate this way. Can you please add a TODO for this ?

It is fine if testing triggered the upgrades. Just ensure VCDMachineTemplate object is updated first before MachineDeployment

tests/e2e/utils/cluster_upgrade_utils.go line 425 at r1 (raw file):

	currTkgVersion, err := getTkgVersionFromTKGMap(currK8sVersion, tkgMap)
	if err != nil {
		return "", fmt.Errorf("error retrieving TKG version from capiyaml: [%v]", err)

error retrieving TKG version for a given K8s version [%s] from the tkgmap

tests/e2e/utils/cluster_upgrade_utils.go line 428 at r1 (raw file):

	}

	supportedUpgrades, err := getSupportedUpgrades(currK8sVersion, currTkgVersion, tkgMap)

I am unsure if we need to get the entire supported upgrade list, we could pass the target tkg version and target K8s version also into just one method, and the output could be tkgmapKey and vAppTemplate.

I don't see a need for two methods, "getSupportedUpgrades" and "getRdeUpgradeResources".

It is okay for now, please create a task to refactor this. Please don't hesitate to chat if you think I am missing something.

We just want one reusable method

getTKGInfoForAGivenK8sAndTKGVersion // get all the tkg details for a given K8s and tkg version
// parses through entire tkg json and outputs the map (tkg details)

And then see if that ova exists in vApp templates.

tests/e2e/utils/cluster_upgrade_utils.go line 439 at r1 (raw file):

	updatedK8sVersion, err := getCapiYamlAfterUpgrade(ctx, r, capiYaml, tkgMapKey, tkgMap, vappTemplate)
	if err != nil {
		return "", fmt.Errorf("error constructing new capiyaml: [%v]", err)

error applying the modified objects of KCP, MD, VCDMachineTemplate for upgrade operation

tests/e2e/utils/cluster_upgrade_utils.go line 454 at r1 (raw file):

	testClient *testingsdk.TestClient, runtimeClient runtimeclient.Client, targetK8sVersion string,
) error {
	timeout := timeoutMinutes * time.Minute

what are the timeout and poll interval values? What is the value of timeoutMinutes?

tests/e2e/utils/cluster_upgrade_utils.go line 459 at r1 (raw file):

	return wait.PollImmediate(pollInterval, timeout, func() (bool, error) {
		// getting all nodes
		nodeList, err := testClient.GetNodes(context.Background())

Is there a need to get the node details?

I think we can just wait on the machines.
Machines also has K8 version as one of its columns.

if every machine has target k8s version and is in running state, that is good enough check.Please verify

Signed-off-by: ymo24 <[email protected]>

ymo24

Reviewable status: 4 of 7 files reviewed, 12 unresolved discussions (waiting on @Anirudh9794, @rliang88, and @sahithi)

tests/e2e/e2e_suite_test.go line 13 at r1 (raw file):