Skip to content
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

feat: add resource snapshot info in the crp status #589

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apis/placement/v1beta1/clusterresourceplacement_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,11 @@ type ClusterResourcePlacementStatus struct {
// +optional
SelectedResources []ResourceIdentifier `json:"selectedResources,omitempty"`

// ClusterResourceSnapshots is a list of clusterResourceSnapshot names.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this only contain the most recent resource snapshots or all existing resource snapshots for this placement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this only contains the current resource snapshots.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, be explicit in the comment.

// ClusterResourceSnapshot is used to store a snapshot of selected resources by a resource placement policy.
// A placement could produce multiple clusterResourceSnapshots to select large amount of resources.
ClusterResourceSnapshots []string `json:"clusterResourceSnapshots,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the PlacementStatuses reflect the status of the listed ClusterResourceSnapshots or they can correspond to a previous version of the snapshots?

Basically, will we produce 2?

  1. snapshot1, placementstatus for snapshot1
  2. snapshot2, placementstatus for snapshot1
  3. snapshot2, placementstatus for snapshot2


// 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
Expand Down
7 changes: 6 additions & 1 deletion apis/placement/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,14 @@ spec:
status:
description: The observed status of ClusterResourcePlacement.
properties:
clusterResourceSnapshots:
description: ClusterResourceSnapshots is a list of clusterResourceSnapshot
names. ClusterResourceSnapshot is used to store a snapshot of selected
resources by a resource placement policy. A placement could produce
multiple clusterResourceSnapshots to select large amount of resources.
items:
type: string
type: array
conditions:
description: Conditions is an array of current observed conditions
for ClusterResourcePlacement.
Expand Down
Loading