search

kubestellar / kubestellar

KubeStellar - a flexible solution for challenges associated with multi-cluster configuration management for edge, multi-cloud, and hybrid cloud
https://kubestellar.io
Apache License 2.0
264 stars 60 forks source link

bug: assign correctly placement labels in transport manifest when different placements select overlapping groups of clusters #1541

Closed pdettori closed 8 months ago

pdettori commented 8 months ago

Describe the bug

The current logic for assigning placement labels for an object that has overlapping placement with overlapping matching clusters is not correct. This is because of a shortcut that was taken in the placement logic implementation.

For example, object 1 has matching placement 1, that selects clusters 1 and 2, and placement 2, that selects clusters 2 and 3. The list of clusters for the object is cluster 1, cluster 2, cluster 3. The shortcut implementation adds the label for placement 1 and 2 to all copies of the manifest in cluster 1, 2 and 3.

Steps To Reproduce

Test by creating overlapping placements as described in the bug, and examine the labels in the ManifestWork.

Expected Behavior

the correct implementation should add the label for placement 1 on cluster 1, for placement 1 and 2 on cluster 2, and for placement 2 on cluster 3.

Additional Context

No response

nirrozenbaum commented 8 months ago

@pdettori can you please explain why do you need the labels? is it done in order not to propagate the same object multiple times (every instance in different manifest work)?

pdettori commented 8 months ago

@nirrozenbaum it is part of the delete logic when a placement is deleted. See https://ibm.ent.box.com/file/1389615774191 in the "Placement Deleted" section.

nirrozenbaum commented 8 months ago

@pdettori I see. this is no longer valid with the new design, so this bug should be automatically resolved once the new implementation is merged.

please notice that when you wrap every single object there are pros and cons. from one hand as you mentioned in previous discussions same object is not propagated multiple times in different wrapped objects. on the other hand, this opens the door for many potential issues like the bugs we're seeing people are reporting recently. I think we should keep things simple. if object was inserted into two different placement objects, it will appear in two different wrapped objects. I don't see any issue with that as it is the exact same object so there are no conflicts. it is true that it's not optimal (we can propagate the specific object only once) but I think the tradeoff doesn't worth it as it complicates code and maintenance (for example this label management which could be removed).

having said the above, I'll leave the discussion on this topic to a followup meeting regarding the internal details of the transport. we'll start with something that works and make progress from there.

pdettori commented 8 months ago

@nirrozenbaum I think we need more detailed design discussions, There are still many questions I'd like to go over to compare the pros and cons of each solution. Some questions I mentioned in previous discussions:

  1. How to handle overlapping placements: the same object could be targeted by two different placements. If there is say object o targeted by placement an and by placement b, is there going to be a placement decision for placement an and one for placement b with o inside ? Suppose placement a targets cluster 1, 2 and placement b target cluster 2 and 3. If there is an overlap in cluster 2 (as in this example) would the transport controller watching placement decisions write the wrapped object to cluster 2 namespace twice?

It looks from your answer above that this would indeed be the case. Would that create any conflict on the Work-Agent side? How can we minimize this potential overlap from user perspective ? I can see that these overlaps would increase the amount of data to be sent downstream over the network, which may have some impact for edge clusters with low BW connectivity.

  1. How to handle conflicts for objects with same name, namespace, api group, version and resource name that are created in two different WDSs (and are different objects). How the conflict can be detected if bundling all objects of a placement together? See this discussion for more context: https://github.com/kubestellar/kubestellar/issues/1538

  2. How the update of a single object is handled. Suppose that you create deployment in the WDS. The placement controller writes the placement decision, the transport controller sees there is a new (or updated) placement decision, it wraps it up and deliver it to the mailbox namespace. Perhaps there are also other objects in that placement decision that are targeted by the placement. Now, assume that the deployment is updated - e.g. changing the number of replicas from 1 to 3. How the transport is informed of the change in spec of the deployment? Would placement update something in placement decision? And if yes, would that force the transport controller to re-write all objects in that placement decision to the mailbox namespaces?

  3. How to handle singleton status (this is a required feature we need with MCAD as the MCAD dispatcher requires full status in the AppWrapper to work - much more important at this point that upsync). Currently, this is implemented with an OCM Add-On for pushing back to the OCM mailbox namespace the full status, wrapped in a WorkStatus CR. That CR currently matches the ManifestWork and is updated only for one object (because Manifest only contains one object). Need to think what are the implications when moving from a single object to a multi-object Manifest.

nirrozenbaum commented 8 months ago

@pdettori I'll set a dedicated 1h discussion to discuss the above points.