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.

Sign up for GitHub

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: updated ResourceBinding API #400

Merged
merged 6 commits into from
Jun 29, 2023
Merged

interface: updated ResourceBinding API #400

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e45fd3d
API change
michaelawyu Jun 28, 2023
77ba55d
Update apis/placement/v1beta1/binding_types.go
ryanzhang-oss Jun 28, 2023
8c976d4
Update apis/placement/v1beta1/binding_types.go
ryanzhang-oss Jun 28, 2023
e695981
Update apis/placement/v1beta1/binding_types.go
ryanzhang-oss Jun 28, 2023
e2c1c8e
Update apis/placement/v1beta1/binding_types.go
ryanzhang-oss Jun 28, 2023
88fe777
Re-gen the manifests
michaelawyu Jun 29, 2023
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
16 changes: 16 additions & 0 deletions apis/placement/v1beta1/binding_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,16 @@ type ResourceBindingSpec struct {
// it points to the name of the leading snapshot of the index group.
ResourceSnapshotName string `json:"resourceSnapshotName"`

// SchedulingPolicySnapshotName is the name of the scheduling policy snapshot that this resource binding
// points to; more specifically, the scheduler creates this bindings in accordance with this
// scheduling policy snapshot.
SchedulingPolicySnapshotName string `json:"schedulingPolicySnapshotName"`

// TargetCluster is the name of the cluster that the scheduler assigns the resources to.
TargetCluster string `json:"targetCluster"`

// ClusterDecision explains why the scheduler selected this cluster.
ClusterDecision ClusterDecision `json:"clusterDecision"`
}

// BindingState is the state of the binding.
Expand Down Expand Up @@ -83,6 +91,14 @@ const (
// - "Unknown" means it is unknown.
ResourceBindingBound ResourceBindingConditionType = "Bound"

// ResourceBindingUpdated indicates whether (and when) a binding is updated to point
// to a new resource snapshot.
// Its condition status can be one of the following:
// - "True" means the binding has been updated.
// - "False" means the binding has not been updated yet.
// - "Unknown" means the update status is unknown.
ResourceBindingUpdated ResourceBindingConditionType = "Updated"

// ResourceBindingApplied indicates the applied condition of the given resources.
// Its condition status can be one of the following:
// - "True" means all the resources are created in the target cluster.
Expand Down
4 changes: 0 additions & 4 deletions apis/placement/v1beta1/resourcesnapshot_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ type ResourceSnapshotSpec struct {
// SelectedResources contains a list of resources selected by ResourceSelectors.
// +required
SelectedResources []ResourceContent `json:"selectedResources"`

// PolicySnapshotName is the name of the policy snapshot that this resource snapshot is pointing to.
// +required
PolicySnapshotName string `json:"policySnapshotName"`
Copy link
Contributor

Choose a reason for hiding this comment

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

just wonder why we remove this field?

For example, CRP has two policySnapshots & two resourceSnapshots. policySnapshot A selects resources A and policy Snapshot B selects resource B. policySnapshot B and resourceSnapshot B are active.

Scheduler has not created the new bindings for policyA and marked the old bindings as deleted yet.
In this case, should rollout controller avoid updating the these old bindings by checking the policySnapshotName in the resourceSnapshot?

If we remove this field, we cannot distinguish whether the resourceSnapshot is created for policyA or policyB.

For the future rollback feature, should we also rollback the policy?

Copy link
Contributor

Choose a reason for hiding this comment

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

The main reason we removed this field is to completely decouple the schedulingPolicy from resource.

Scheduler has not created the new bindings for policyA and marked the old bindings as deleted yet.
In this case, should rollout controller avoid updating the these old bindings by checking the policySnapshotName in the resourceSnapshot?

I am not sure if I follow this case. The policyA is old so why is scheduler creating new bindings for it? What are the old bindings for?

If we remove this field, we cannot distinguish whether the resourceSnapshot is created for policyA or policyB.
Isn't the latest resourceSnapshot always point to the latest policy?

For the future rollback feature, should we also rollback the policy?
Given now that we have two revisions. It seems that the future rollback policy has to contain both of them?

}

// ResourceContent contains the content of a resource
Expand Down
5 changes: 3 additions & 2 deletions apis/placement/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

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

50 changes: 50 additions & 0 deletions config/crd/bases/placement.azure.com_clusterresourcebindings.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,54 @@ spec:
spec:
description: The desired state of ClusterResourceBinding.
properties:
clusterDecision:
description: ClusterDecision explains why the scheduler makes this
binding.
properties:
clusterName:
description: ClusterName is the name of the ManagedCluster. If
it is not empty, its value should be unique cross all placement
decisions for the Placement.
type: string
clusterScore:
description: ClusterScore represents the score of the cluster
calculated by the scheduler.
properties:
affinityScore:
description: AffinityScore represents the affinity score of
the cluster calculated by the last scheduling decision based
on the preferred affinity selector. An affinity score may
not present if the cluster does not meet the required affinity.
format: int32
type: integer
priorityScore:
description: TopologySpreadScore represents the priority score
of the cluster calculated by the last scheduling decision
based on the topology spread applied to the cluster. A priority
score may not present if the cluster does not meet the topology
spread.
format: int32
type: integer
type: object
reason:
description: Reason represents the reason why the cluster is selected
or not.
type: string
selected:
description: Selected indicates if this cluster is selected by
the scheduler.
type: boolean
required:
- clusterName
- reason
- selected
type: object
policySnapshotName:
description: PolicySnapshotName is the name of the scheduling policy
snapshot that this resource binding points to; more specifically,
the scheduler creates this bindings in accordance with this scheduling
policy snapshot.
type: string
resourceSnapshotName:
description: ResourceSnapshotName is the name of the resource snapshot
that this resource binding points to. If the resources are divided
Expand All @@ -62,6 +110,8 @@ spec:
assigns the resources to.
type: string
required:
- clusterDecision
- policySnapshotName
- resourceSnapshotName
- state
- targetCluster
Expand Down
10 changes: 5 additions & 5 deletions config/crd/bases/placement.azure.com_clusterresourceplacements.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,8 @@ spec:
- type: string
description: 'The maximum number of clusters that can be scheduled
above the desired number of clusters. The desired number
equal to the `NumberOfClusters` field when the placement
type is `PickN`. The desired number equal to the number
equals to the `NumberOfClusters` field when the placement
type is `PickN`. The desired number equals to the number
of clusters scheduler selected when the placement type is
`PickAll`. Value can be an absolute number (ex: 5) or a
percentage of desire (ex: 10%). Absolute number is calculated
Expand All @@ -426,9 +426,9 @@ spec:
- type: string
description: 'The maximum number of clusters that can be unavailable
during the rolling update comparing to the desired number
of clusters. The desired number equal to the `NumberOfClusters`
of clusters. The desired number equals to the `NumberOfClusters`
field when the placement type is `PickN`. The desired number
equal to the number of clusters scheduler selected when
equals to the number of clusters scheduler selected when
the placement type is `PickAll`. Value can be an absolute
number (ex: 5) or a percentage of the desired number of
clusters (ex: 10%). Absolute number is calculated from percentage
Expand All @@ -437,7 +437,7 @@ spec:
resources content on the same cluster. This can not be 0
if MaxSurge is 0. Defaults to 25%.'
x-kubernetes-int-or-string: true
waitBetweenRollingPeriodSeconds:
unavailablePeriodSeconds:
description: UnavailablePeriodSeconds is used to config the
time to wait between rolling out phases. A resource placement
is considered available after `UnavailablePeriodSeconds`
Expand Down
5 changes: 0 additions & 5 deletions config/crd/bases/placement.azure.com_clusterresourcesnapshots.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ spec:
spec:
description: The desired state of ResourceSnapshot.
properties:
policySnapshotName:
description: PolicySnapshotName is the name of the policy snapshot
that this resource snapshot is pointing to.
type: string
selectedResources:
description: SelectedResources contains a list of resources selected
by ResourceSelectors.
Expand All @@ -74,7 +70,6 @@ spec:
x-kubernetes-preserve-unknown-fields: true
type: array
required:
- policySnapshotName
- selectedResources
type: object
status:
Expand Down