-
Notifications
You must be signed in to change notification settings - Fork 20
interface: progress drift/diff detection APIs to v1 #298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
interface: progress drift/diff detection APIs to v1 #298
Conversation
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
|
Can be reviewed; though I am still finishing up some manual testing. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…o interface/progress-drift-diff-to-v1
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
|
Note: some of the additional tests will be submitted in separate PRs in an attempt to control the PR size. |
| lessFuncPlacementStatus = func(a, b placementv1beta1.PerClusterPlacementStatus) bool { | ||
| return a.ClusterName < b.ClusterName | ||
| } | ||
| lessFuncPlacementStatusV1 = func(a, b placementv1.ResourcePlacementStatus) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this? There is no ResourcePlacementStatus in v1beta1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Ryan! In the v1 package it is the type of the ClusterResourcePlacementStatus.PlacementStatuses field, i.e.,
// ClusterResourcePlacementStatus defines the observed state of the ClusterResourcePlacement object.
type ClusterResourcePlacementStatus struct {
...
// PlacementStatuses contains a list of placement status on the clusters that are selected by PlacementPolicy.
// Each selected cluster according to the latest resource placement is guaranteed to have a corresponding placementStatuses.
// In the pickN case, there are N placement statuses where N = NumberOfClusters; Or in the pickFixed case, there are
// N placement statuses where N = ClusterNames.
// In these cases, some of them may not have assigned clusters when we cannot fill the required number of clusters.
// TODO, For pickAll type, considering providing unselected clusters info.
// +optional
PlacementStatuses []ResourcePlacementStatus `json:"placementStatuses,omitempty"`
...
}It has nothing to do with the ResourcePlacement API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In v1beta1 package the field and the type has been renamed to PerClusterPlacementStatus, but the JSON serialization is left unchanged.
|
Merging this now to unblock progress; if there's any concern, please let me know. |
Description of your changes
This PR progresses drift/diff detection APIs to v1 version.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
N/A; pending some manual testing.
Special notes for your reviewer
N/A