restore release version 1.126.0

[xiaoweim]

Change description

Fixes #

Tests you have done

• Run make ready-pr to ensure this PR is ready for review.
• Perform necessary E2E testing for changed resources.

[xiaoweim]

/assign @justinsb

[justinsb]

/approve
/lgtm

/hold for @yuwenma to approve

[xiaoweim]

/assign @yuwenma

[justinsb]

A few things @cheftako pointed out to me:

1. We don't want a "gap" of versions, we should have at least each minor version in the window available here (c.f. https://github.com/GoogleCloudPlatform/k8s-config-connector/tree/2a5c056279d917232bc9deaa1f0ce02da025bb5f/operator/autopilot-channels/packages/configconnector ) Whether we want to support minor patch versions IDK, I would guess "any versions we shipped" is going to be the easiest policy

2. We should be careful about whether downgrade is safe. You're not introducing this here so we can likely treat this as a separate issue. Walter was worried about ClusterRoles / Roles; I think I'm more worried about CRD removal or field removal (via downgrade). We're introducing more testing into the operator so I think we can deal with this there ( tests: add golden tests for operator #4528 )

[yuwenma]

Could you share more context about this change? From the discussion, my read as we want to keep all the passed releases until the latest one in GKE add-on, which means the manifests contain 1.126.0, 1.127.0, 1.128.0, 1.129.2, 1.130.0, 1.131.0.

[xiaoweim]

I just updated the PR to not leave a gap in between older versions and restored all the shipped versions up until 1.126.0

[jingyih]

I am wondering if we could only add back the per-namespace-components.yaml files from the previous verions?

It looks like we are only using version from CCC to load per namespace components. [1]

This way it is more clear that the rollback does not include CRDs and ClusterRoels.

[1]

k8s-config-connector/operator/pkg/manifest/per_namespace_manifest_loader.go

Line 63 in 3132f8d

files, err := p.repo.LoadNamespacedComponents(ctx, componentName, version)

[xiaoweim]

I am wondering if we could only add back the per-namespace-components.yaml files from the previous verions?

It looks like we are only using version from CCC to load per namespace components. [1]

This way it is more clear that the rollback does not include CRDs and ClusterRoels.

[1]

k8s-config-connector/operator/pkg/manifest/per_namespace_manifest_loader.go

Line 63 in 3132f8d

files, err := p.repo.LoadNamespacedComponents(ctx, componentName, version)

I think that is a good idea. Let me update the PR!

[jingyih]

I just realized we also have 1.129 and 1.130 folders that include everything. For consistency, we should either remove the CRDs and ClusterRoles from those folders or add them back for every version. Keeping only the per-namespace file clarifies what happens during a downgrade, but it may confuse users browsing the release folders as they'll see only one file. I’ll defer to @yuwenma on what we want to include in each version.

[yuwenma]

My understanding is that we need to contain both 0-cnrm-system.yaml and per-namespace-components.yaml for each namespace version. Because user can choose a stale release as their very first installation in which case the namespace and RBAC will be missing. For example, user choose to use the 1.132 bundle to specifically install 1.126 (via CCC spec.version), there could be a chance that only the ./packages/1.126 are applied, in which case the 0-cnrm-system.yaml will be missing. Or does the CCC/CC specifically install this file in some other steps?

[yuwenma]

/assign @cheftako

[xiaoweim]

My understanding is that we need to contain both 0-cnrm-system.yaml and per-namespace-components.yaml for each namespace version. Because user can choose a stale release as their very first installation in which case the namespace and RBAC will be missing. For example, user choose to use the 1.132 bundle to specifically install 1.126 (via CCC spec.version), there could be a chance that only the ./packages/1.126 are applied, in which case the 0-cnrm-system.yaml will be missing. Or does the CCC/CC specifically install this file in some other steps?

My understanding is that when we have specify the version field in the CCC, we load the version specific manifest but only looking for packages/${version}/namespaced/per-namespace-components.yaml https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/operator/pkg/manifest/repo.go#L115.

I did not quite get where the user chooses to use 1.132.0 bundle to install 1.126.0. IIUC if the user install KCC 1.132 version first, and then update the CCC Version field to 1.126.0, their cluster should already have CRD and RBAC permissions installed and only loads the /per-namespace-components.yaml manifest.

[xiaoweim]

I have just updated the PR to include the current CRD and RBAC into previous KCC release versions with old operators.

[yuwenma]

/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, yuwenma

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 [justinsb,yuwenma]

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

[yuwenma]

One follow-up suggestion about fixing the propose-tag PR, non blocker to this change. Thanks and good job @xiaoweim

[xiaoweim]

One follow-up suggestion about fixing the propose-tag PR, non blocker to this change. Thanks and good job @xiaoweim

I have created #4646 to update the propose-tag script.

[cheftako]

/lgtm
/hold while Xiaowei checks the CRD files are consistent. (Github makes validating these difficult) Can you just confirm there are not diffs between the copies?

[cheftako]

/unhold after checking with Xiaowei

[cheftako]

/remove hold

[xiaoweim]

/unhold