fix: Add fleet annotation to namespace of CAPI cluster

[yiannistri]

This is part of a fix for rancher/turtles#1329
Fixes: #279

This PR adds a reconciler that ensures a fleet annotation (rancher/webhook#932) is present in namespaces of CAPI clusters.

[Danil-Grigorev]

Thanks @yiannistr, that looks great. Couple of comments

[Danil-Grigorev]

Looks good, but I think this can also be a part of to_bundle method in

cluster-api-addon-provider-fleet/src/controllers/cluster.rs

Lines 226 to 235 in 5aaff1b

	Ok(Some(FleetClusterBundle {
	template_sources: TemplateSources::new(self),
	fleet: self.to_cluster(config.spec.cluster.as_ref()),
	fleet_group: self.to_group(config.spec.cluster.as_ref()),
	mapping: self.to_bundle_ns_mapping(config.spec.cluster.as_ref()),
	#[cfg(feature = "agent-initiated")]
	cluster_registration_token: self
	.to_cluster_registration_token(config.spec.cluster.as_ref()),
	config,
	}))

Just needs to add a Namespace object to the FleetClusterBundle, and then use patch method on it as on the rest of the bundle objects. Will need a ResourceDiff implementation for Namespace though (we only care about annotations there).

[Danil-Grigorev]

Suggested change

	// List all other clusters in this namespace
	let namespaces = Api::<Namespace>::all(ctx.client.clone());
	let namespace_name = mapping.namespace().unwrap_or_default();
	let clusters = Api::<Cluster>::namespaced(ctx.client.clone(), &namespace_name);
	let other_clusters = clusters
	.list(&ListParams::default().fields(&format!("metadata.namespace={},metadata.name!={}", namespace_name, self.fleet.name_any())))
	.await?;

I think other_clusters can be cloned and reused, to avoid additional list request, as you do down below. That may require moving this outside of if let Some(mapping) block, otherwise this logic will only be executed on a cluster referencing a cluster-class from another namespace, which I don’t think is the goal here.

[yiannistri]

@Danil-Grigorev doesn'tother_clusters use the namespace of the BundleNamespaceMapping as opposed to the namespace of the cluster?

cluster-api-addon-provider-fleet/src/controllers/cluster.rs

Lines 184 to 191 in 5aaff1b

	let ns = mapping.namespace();
	let other_clusters = ctx
	.client
	.list::<Cluster>(
	&ListParams::default(),
	&scope::Namespace::from(ns.clone().unwrap_or_default()),
	)
	.await?;

[Danil-Grigorev]

Yes, you are right, thanks, it shouldn’t be the ns of bundle ns mapping but a cluster or a fleet cluster instead (got misled by the code a bit). Still, this code block should be moved outside of let Some( case.

Maybe also a simple addition to e2e would be good here? Something in a sense of kubectl wait namespace <ns> json-path=<annotation-present>. This functionality is quite critical and would allow users to manually create FleetWorkspace on demand from UI or API, even in case of no further development in the repo. This should close rancher/turtles#1329 as a best effort.