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

SyncTarget Uniqueness #1600

Merged
Merged
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ require (
github.com/kcp-dev/apimachinery v0.0.0-20220719204446-c2de90c1e6ac
github.com/kcp-dev/kcp/pkg/apis v0.0.0-00010101000000-000000000000
github.com/kcp-dev/logicalcluster/v2 v2.0.0-alpha.0
github.com/martinlindhe/base36 v1.1.1
github.com/muesli/reflow v0.1.0
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
github.com/sirupsen/logrus v1.8.1
Expand Down Expand Up @@ -101,6 +100,7 @@ require (
github.com/json-iterator/go v1.1.12 // indirect
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
github.com/mailru/easyjson v0.7.6 // indirect
github.com/martinlindhe/base36 v1.1.1 // indirect
github.com/mattn/go-colorable v0.1.8 // indirect
github.com/mattn/go-isatty v0.0.12 // indirect
github.com/mattn/go-runewidth v0.0.7 // indirect
Expand Down
23 changes: 21 additions & 2 deletions pkg/apis/workload/v1alpha1/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,31 @@ limitations under the License.
package v1alpha1

import (
"crypto/sha256"
"math/big"

"github.com/kcp-dev/logicalcluster/v2"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// GetResourceState returns the state of the resource for the given sync target, and
// whether the state value is a valid state. A missing label is considered invalid.
func GetResourceState(obj metav1.Object, cluster string) (state ResourceState, valid bool) {
value, found := obj.GetLabels()[ClusterResourceStateLabelPrefix+cluster]
func GetResourceState(obj metav1.Object, syncTargetKey string) (state ResourceState, valid bool) {
value, found := obj.GetLabels()[ClusterResourceStateLabelPrefix+syncTargetKey]
return ResourceState(value), found && (value == "" || ResourceState(value) == ResourceStateSync)
}

// ToSyncTargetKey hashes the SyncTarget workspace and the SyncTarget name to a string that is used to idenfity
// in a unique way the synctarget in annotations/labels/finalizers.
func ToSyncTargetKey(syncTargetWorkspace logicalcluster.Name, syncTargetName string) string {
hash := sha256.Sum224([]byte(syncTargetWorkspace.String() + syncTargetName))
base62hash := toBase62(hash)
return base62hash
}

func toBase62(hash [28]byte) string {
var i big.Int
i.SetBytes(hash[:])
return i.Text(62)
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (

schedulingv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/scheduling/v1alpha1"
workloadv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/workload/v1alpha1"
placementreconciler "github.com/kcp-dev/kcp/pkg/reconciler/workload/placement"
)

const removingGracePeriod = 5 * time.Second
Expand Down Expand Up @@ -73,10 +72,7 @@ func (r *placementSchedulingReconciler) reconcile(ctx context.Context, ns *corev
if !foundScheduled {
continue
}

// TODO: location workspace should be considered also
_, syncTarget := placementreconciler.ParseCurrentScheduled(currentScheduled)
scheduledSyncTargets.Insert(syncTarget)
scheduledSyncTargets.Insert(currentScheduled)
}

// 2. find the scheduled synctarget to the ns, including synced, removing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestScheduling(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
},
{
Expand All @@ -87,15 +87,15 @@ func TestScheduling(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
placement: newPlacement("test-placement", "test-location", "test-cluster"),
wantPatch: false,
expectedAnnotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
},
{
Expand All @@ -104,16 +104,16 @@ func TestScheduling(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
placement: newPlacement("test-placement", "test-location", ""),
wantPatch: true,
expectedAnnotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "test-cluster": now3339,
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": now3339,
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
},
{
Expand All @@ -122,46 +122,46 @@ func TestScheduling(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
placement: newPlacement("test-placement", "test-location", "test-cluster-2"),
wantPatch: true,
expectedAnnotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "test-cluster": now3339,
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": now3339,
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster-2": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQA9mRmZ5RuT9vKRZokxZTm1Yk9SqKyfOMoTEr": string(workloadv1alpha1.ResourceStateSync),
},
},
{
name: "scheduled cluster is removing",
annotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "test-cluster": now3339,
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": now3339,
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
placement: newPlacement("test-placement", "test-location", "test-cluster"),
wantPatch: false,
expectedAnnotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "test-cluster": now3339,
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": now3339,
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
},
{
name: "remove clusters which is removing after grace period",
annotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "test-cluster": time.Now().Add(-1 * (removingGracePeriod + 1)).UTC().Format(time.RFC3339),
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": time.Now().Add(-1 * (removingGracePeriod + 1)).UTC().Format(time.RFC3339),
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "test-cluster": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "34sZi3721YwBLDHUuNVIOLxuYp5nEZBpsTQyDq": string(workloadv1alpha1.ResourceStateSync),
},
placement: newPlacement("test-placement", "test-location", ""),
wantPatch: true,
Expand Down Expand Up @@ -235,8 +235,8 @@ func TestMultiplePlacements(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c1": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c2": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": string(workloadv1alpha1.ResourceStateSync),
},
},
{
Expand All @@ -253,7 +253,7 @@ func TestMultiplePlacements(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c1": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": string(workloadv1alpha1.ResourceStateSync),
},
},
{
Expand All @@ -266,16 +266,16 @@ func TestMultiplePlacements(t *testing.T) {
schedulingv1alpha1.PlacementAnnotationKey: "",
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c1": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c2": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": string(workloadv1alpha1.ResourceStateSync),
},
wantPatch: false,
expectedAnnotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c1": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c2": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": string(workloadv1alpha1.ResourceStateSync),
},
},
{
Expand All @@ -285,25 +285,25 @@ func TestMultiplePlacements(t *testing.T) {
newPlacement("p2", "loc2", "c4"),
},
annotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "c1": now3339,
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "c2": now3339,
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": now3339,
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": now3339,
},
labels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c1": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c2": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": string(workloadv1alpha1.ResourceStateSync),
},
wantPatch: true,
expectedAnnotations: map[string]string{
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "c1": now3339,
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "c2": now3339,
schedulingv1alpha1.PlacementAnnotationKey: "",
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": now3339,
workloadv1alpha1.InternalClusterDeletionTimestampAnnotationPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": now3339,
},
expectedLabels: map[string]string{
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c1": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c2": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c3": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "c4": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "5iSfzYTm7pPirj6HKlmfvXMb6AuqSBxNB7vkVP": string(workloadv1alpha1.ResourceStateSync),
workloadv1alpha1.ClusterResourceStateLabelPrefix + "8s5f69zIcmjG486nG2jBF8BdYtgwPS7PVP1bTL": string(workloadv1alpha1.ResourceStateSync),
},
},
}
Expand Down Expand Up @@ -356,7 +356,7 @@ func newPlacement(name, location, synctarget string) *schedulingv1alpha1.Placeme

if len(synctarget) > 0 {
placement.Annotations = map[string]string{
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: fmt.Sprintf("%s/%s", location, synctarget),
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: workloadv1alpha1.ToSyncTargetKey(logicalcluster.New(""), synctarget),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ package placement
import (
"context"
"encoding/json"
"fmt"
"math/rand"
"strings"

"github.com/kcp-dev/logicalcluster/v2"

Expand Down Expand Up @@ -67,15 +65,11 @@ func (r *placementSchedulingReconciler) reconcile(ctx context.Context, placement

// 2. do nothing if scheduled cluster is in the valid clusters
if foundScheduled && len(syncTargets) > 0 {
scheduledSyncTargeClusterName, scheduledSyncTargeName := ParseCurrentScheduled(currentScheduled)
for _, syncTarget := range syncTargets {
if syncTargetClusterName != scheduledSyncTargeClusterName {
syncTargetKey := workloadv1alpha1.ToSyncTargetKey(logicalcluster.From(syncTarget), syncTarget.Name)
if syncTargetKey != currentScheduled {
continue
}
if scheduledSyncTargeName != syncTarget.Name {
continue
}

return reconcileStatusContinue, placement, nil
}
}
Expand All @@ -86,7 +80,7 @@ func (r *placementSchedulingReconciler) reconcile(ctx context.Context, placement
// to be exclusive.
if len(syncTargets) > 0 {
scheduledSyncTarget := syncTargets[rand.Intn(len(syncTargets))]
expectedAnnotations[workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey] = fmt.Sprintf("%s/%s", syncTargetClusterName.String(), scheduledSyncTarget.Name)
expectedAnnotations[workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey] = workloadv1alpha1.ToSyncTargetKey(syncTargetClusterName, scheduledSyncTarget.Name)
updated, err := r.patchPlacementAnnotation(ctx, clusterName, placement, expectedAnnotations)
return reconcileStatusContinue, updated, err
}
Expand Down Expand Up @@ -147,11 +141,3 @@ func (r *placementSchedulingReconciler) patchPlacementAnnotation(ctx context.Con
}
return updated, nil
}

func ParseCurrentScheduled(value string) (logicalcluster.Name, string) {
if len(value) == 0 {
return logicalcluster.Name{}, ""
}
comps := strings.SplitN(value, "/", 2)
return logicalcluster.New(comps[0]), comps[1]
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package placement
import (
"context"
"encoding/json"
"fmt"
"testing"

jsonpatch "github.com/evanphx/json-patch"
Expand Down Expand Up @@ -64,7 +63,7 @@ func TestSchedulingReconcile(t *testing.T) {
syncTargets: []*workloadv1alpha1.SyncTarget{newSyncTarget("c1", true)},
wantPatch: true,
expectedAnnotations: map[string]string{
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: "/c1",
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa",
},
},
{
Expand All @@ -73,7 +72,7 @@ func TestSchedulingReconcile(t *testing.T) {
location: newLocation("test-location"),
syncTargets: []*workloadv1alpha1.SyncTarget{newSyncTarget("c1", true)},
expectedAnnotations: map[string]string{
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: "/c1",
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: "aQtdeEWVcqU7h7AKnYMm3KRQ96U4oU2W04yeOa",
},
},
{
Expand All @@ -91,7 +90,7 @@ func TestSchedulingReconcile(t *testing.T) {
syncTargets: []*workloadv1alpha1.SyncTarget{newSyncTarget("c1", false), newSyncTarget("c2", true)},
wantPatch: true,
expectedAnnotations: map[string]string{
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: "/c2",
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: "aPkhvUbGK0xoZIjMnM2pA0AuV1g7i4tBwxu5m4",
},
},
}
Expand Down Expand Up @@ -154,7 +153,7 @@ func newPlacement(name, location, synctarget string) *schedulingv1alpha1.Placeme

if len(synctarget) > 0 {
placement.Annotations = map[string]string{
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: fmt.Sprintf("/%s", synctarget),
workloadv1alpha1.InternalSyncTargetPlacementAnnotationKey: workloadv1alpha1.ToSyncTargetKey(logicalcluster.New(""), synctarget),
}
}

Expand Down
Loading