Skip to content

Commit

Permalink
Changes UpdateLabels to Patch instead of Update
Browse files Browse the repository at this point in the history
  • Loading branch information
@igooch
igooch committed Apr 24, 2024
1 parent 2e34147 commit 4c68627
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 23 deletions.
10 changes: 8 additions & 2 deletions pkg/sdkserver/sdkserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -452,7 +453,7 @@ func (s *SDKServer) gameServer() (*agonesv1.GameServer, error) {
// updateLabels updates the labels on this GameServer to the ones persisted in SDKServer,
// i.e. SDKServer.gsLabels, with the prefix of "agones.dev/sdk-"
func (s *SDKServer) updateLabels(ctx context.Context) error {
s.logger.WithField("labels", s.gsLabels).Debug("Updating label")
s.logger.WithField("labels", s.gsLabels).Debug("Patching label")
gs, err := s.gameServer()
if err != nil {
return err
Expand All @@ -469,7 +470,12 @@ func (s *SDKServer) updateLabels(ctx context.Context) error {
}
s.gsUpdateMutex.RUnlock()

_, err = s.gameServerGetter.GameServers(s.namespace).Update(ctx, gsCopy, metav1.UpdateOptions{})
patch, err := gs.Patch(gsCopy)
if err != nil {
return err
}

_, err = s.gameServerGetter.GameServers(s.namespace).Patch(ctx, gs.GetObjectMeta().GetName(), types.JSONPatchType, patch, metav1.PatchOptions{})
return err
}

Expand Down
146 changes: 125 additions & 21 deletions pkg/sdkserver/sdkserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package sdkserver

import (
"context"
"encoding/json"
"net/http"
"strconv"
"sync"
Expand All @@ -28,6 +29,7 @@ import (
"agones.dev/agones/pkg/sdk/alpha"
agtesting "agones.dev/agones/pkg/testing"
agruntime "agones.dev/agones/pkg/util/runtime"
jsonpatch "github.com/evanphx/json-patch"
"github.com/google/go-cmp/cmp"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
Expand All @@ -44,6 +46,91 @@ import (
testclocks "k8s.io/utils/clock/testing"
)

func TestPatchGameServer(t *testing.T) {

m := agtesting.NewMocks()
initialLabels := map[string]string{"initial": "label"}

gs := agonesv1.GameServer{
ObjectMeta: metav1.ObjectMeta{
Name: "test", Namespace: "default", Generation: 1, Labels: initialLabels,
},
Spec: agonesv1.GameServerSpec{
SdkServer: agonesv1.SdkServer{
LogLevel: "Debug",
},
},
}
gs.ApplyDefaults()

m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil
})

patched := make(chan *agonesv1.GameServer, 10)

m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
pa := action.(k8stesting.PatchAction)
patchJSON := pa.GetPatch()
patch, err := jsonpatch.DecodePatch(patchJSON)
assert.NoError(t, err)
gsJSON, err := json.Marshal(gs)
assert.NoError(t, err)
patchedGs, err := patch.Apply(gsJSON)
assert.NoError(t, err)
err = json.Unmarshal(patchedGs, &gs)
assert.NoError(t, err)
patched <- &gs
return false, &gs, nil
})

ctx, cancel := context.WithCancel(context.Background())
sc, err := defaultSidecar(m)
require.NoError(t, err)
assert.NoError(t, sc.WaitForConnection(ctx))
sc.informerFactory.Start(ctx.Done())
assert.True(t, cache.WaitForCacheSync(ctx.Done(), sc.gameServerSynced))

wg := sync.WaitGroup{}
wg.Add(1)

go func() {
err = sc.Run(ctx)
assert.NoError(t, err)
wg.Done()
}()

// check initial value comes through
require.Eventually(t, func() bool {
gs, err := sc.GetGameServer(ctx, &sdk.Empty{})
assert.NoError(t, err)
return assert.Equal(t, initialLabels, gs.ObjectMeta.Labels)
}, 10*time.Second, time.Second)

// Update the Game Server
_, err = sc.SetLabel(ctx, &sdk.KeyValue{Key: "foo", Value: "value-foo"})
assert.Nil(t, err)

// Confirm update is applied to the SDK Server
expectedLabels := map[string]string{"initial": "label", "agones.dev/sdk-foo": "value-foo"}
require.Eventually(t, func() bool {
gs, err := sc.GetGameServer(ctx, &sdk.Empty{})
assert.NoError(t, err)
return assert.Equal(t, expectedLabels, gs.ObjectMeta.Labels)
}, 10*time.Second, time.Second)

// on an update, confirm that the update hits the K8s api
select {
case value := <-patched:
assert.Equal(t, expectedLabels, value.Labels)
case <-time.After(10 * time.Second):
assert.Fail(t, "game server should have been patched")
}

cancel()
wg.Wait()
}

func TestSidecarRun(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -93,19 +180,19 @@ func TestSidecarRun(t *testing.T) {
recordings: []string{"Warning " + string(agonesv1.GameServerStateUnhealthy)},
},
},
"label": {
f: func(sc *SDKServer, ctx context.Context) {
_, err := sc.SetLabel(ctx, &sdk.KeyValue{Key: "foo", Value: "value-foo"})
assert.Nil(t, err)
_, err = sc.SetLabel(ctx, &sdk.KeyValue{Key: "bar", Value: "value-bar"})
assert.Nil(t, err)
},
expected: expected{
labels: map[string]string{
metadataPrefix + "foo": "value-foo",
metadataPrefix + "bar": "value-bar"},
},
},
// "label": {
// f: func(sc *SDKServer, ctx context.Context) {
// _, err := sc.SetLabel(ctx, &sdk.KeyValue{Key: "foo", Value: "value-foo"})
// assert.Nil(t, err)
// _, err = sc.SetLabel(ctx, &sdk.KeyValue{Key: "bar", Value: "value-bar"})
// assert.Nil(t, err)
// },
// expected: expected{
// labels: map[string]string{
// metadataPrefix + "foo": "value-foo",
// metadataPrefix + "bar": "value-bar"},
// },
// },
"annotation": {
f: func(sc *SDKServer, ctx context.Context) {
_, err := sc.SetAnnotation(ctx, &sdk.KeyValue{Key: "test-1", Value: "annotation-1"})
Expand Down Expand Up @@ -287,13 +374,13 @@ func TestSDKServerSyncGameServer(t *testing.T) {
sc.gsAnnotations = v.scData.gsAnnotations

updated := false
gs := agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{
UID: "1234",
Name: sc.gameServerName, Namespace: sc.namespace,
Labels: map[string]string{}, Annotations: map[string]string{}},
}

m.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
gs := agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{
UID: "1234",
Name: sc.gameServerName, Namespace: sc.namespace,
Labels: map[string]string{}, Annotations: map[string]string{}},
}
return true, &agonesv1.GameServerList{Items: []agonesv1.GameServer{gs}}, nil
})
m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
Expand All @@ -304,16 +391,33 @@ func TestSDKServerSyncGameServer(t *testing.T) {
if v.expected.state != "" {
assert.Equal(t, v.expected.state, gs.Status.State)
}
for label, value := range v.expected.labels {
assert.Equal(t, value, gs.ObjectMeta.Labels[label])
}

for ann, value := range v.expected.annotations {
assert.Equal(t, value, gs.ObjectMeta.Annotations[ann])
}

return true, gs, nil
})

m.AgonesClient.AddReactor("patch", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
pa := action.(k8stesting.PatchAction)
patchJSON := pa.GetPatch()
patch, err := jsonpatch.DecodePatch(patchJSON)
assert.NoError(t, err)
gsJSON, err := json.Marshal(gs)
assert.NoError(t, err)
patchedGs, err := patch.Apply(gsJSON)
assert.NoError(t, err)
err = json.Unmarshal(patchedGs, &gs)
assert.NoError(t, err)

for label, value := range v.expected.labels {
assert.Equal(t, value, gs.ObjectMeta.Labels[label])
}
updated = true
return false, &gs, nil
})

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
sc.informerFactory.Start(ctx.Done())
Expand Down

0 comments on commit 4c68627

Please sign in to comment.