KSPACE-20: Drop the distinction between host & member ToolchainClusters

[fbm3307]

This is related to Drop the distinction between host & member ToolchainClusters
Related PRs:
Host-Operator - codeready-toolchain/host-operator#971
Member-Operator - codeready-toolchain/member-operator#531
Registration-Service - codeready-toolchain/registration-service#402
Toolchain-e2e - codeready-toolchain/toolchain-e2e#893
Sandbox-sre - https://github.com/codeready-toolchain/sandbox-sre/pull/1524

[codecov]

Codecov Report

Merging #359 (99a2215) into master (05f8dfb) will decrease coverage by 0.24%.
The diff coverage is 73.68%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
- Coverage   76.22%   75.99%   -0.24%     
==========================================
  Files          44       44              
  Lines        1708     1683      -25     
==========================================
- Hits         1302     1279      -23     
+ Misses        354      353       -1     
+ Partials       52       51       -1     

Files	Coverage Δ
...rs/toolchaincluster/toolchaincluster_controller.go	45.45% <ø> (-14.55%)	⬇️
pkg/cluster/cache.go	100.00% <100.00%> (ø)
pkg/cluster/service.go	83.46% <100.00%> (-2.25%)	⬇️
pkg/test/cluster.go	0.00% <0.00%> (ø)

[alexeykazakov]

We need to make sure that these changes do not break anything.
Please open PRs which uses this updated common for host, member, reg-service, sandbox-sre and e2e tests.
Be aware of that you can't pair multiple PRs with the same e2e test PR though.

[metlos]

Apart from the comments that @alexeykazakov made, I think it is going to be more important to take a look at how these functions are used in the operators as well as in the unit and e2e tests and only then decide about how to drop the cluster type. I am afraid that the changes outside of toolchain-common are going to be much larger.

[filariow]

mostly LGTM, just some optional suggestions

[mfrancisc]

Thanks for looking at my comments. I have few minor things

Also I was double checking on the usage of cluster.GetHostCluster and cluster.GetMemberClusters and seems like the former just returns one toolchaincluster resource. That's the only difference. So I was wondering if we should have just one generic one ( I think it's something that came out in some other comments in this PR ) . We can do that as a follow up PR if we think it makes sense.

[fbm3307]

Also I was double checking on the usage of cluster.GetHostCluster and cluster.GetMemberClusters and seems like the former just returns one toolchaincluster resource. That's the only difference. So I was wondering if we should have just one generic one ( I think it's something that came out in some other comments in this PR ) . We can do that as a follow up PR if we think it makes sense.

Yes i intend to do that in separate follow-up PR once this is merged

[mfrancisc]

Nice Job 🏅 🚀 👏
Thanks for addressing my comments!

Friendly reminder - please wait also for approval from the other reviewers and approval of the related PRs before merging.

[MatousJobanek]

Looks good, let's just rename one variable to not confuse people

[fbm3307]

Looks good, let's just rename one variable to not confuse people

@MatousJobanek right, updated it. 👍

[MatousJobanek]

cool, thanks for the extra changes. 🚀
Let's start merging the PRs 👍 I believe that you have approval for all of them, just don't forget to update the dependency to toolchain-common as soon as you merge this PR

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
18.1% Duplication on New Code

See analysis details on SonarCloud