Skip to content

Commit

Permalink
This code was for measuring a zalando internal SLO, which we do diffe…
Browse files Browse the repository at this point in the history 
…rently now. We mark the option as deprecated, log message and ignore the option (#1626)

Signed-off-by: Sandor Szücs <[email protected]>
  • Loading branch information
@szuecs
szuecs authored Dec 2, 2020
1 parent e9382c0 commit 9f8c62c
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 190 deletions.
53 changes: 11 additions & 42 deletions dataclients/kubernetes/kube.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"github.com/zalando/skipper/eskip"
"github.com/zalando/skipper/filters/builtin"
"github.com/zalando/skipper/predicates/primitive"
"github.com/zalando/skipper/predicates/source"
)

Expand Down Expand Up @@ -156,7 +155,7 @@ type Options struct {
// The provided filters are then applied to all routes.
DefaultFiltersDir string

// If the OriginMarker should be added as a filter
// OriginMarker is *deprecated* and not used anymore. It will be deleted in v1.
OriginMarker bool

// If the OpenTracing tag containing RouteGroup backend name
Expand All @@ -179,11 +178,13 @@ type Client struct {
sigs chan os.Signal
quit chan struct{}
defaultFiltersDir string
originMarker bool
}

// New creates and initializes a Kubernetes DataClient.
func New(o Options) (*Client, error) {
if o.OriginMarker {
log.Warning("OriginMarker is deprecated")
}
quit := make(chan struct{})

apiURL, err := buildAPIURL(o)
Expand Down Expand Up @@ -254,7 +255,6 @@ func New(o Options) (*Client, error) {
reverseSourcePredicate: o.ReverseSourcePredicate,
quit: quit,
defaultFiltersDir: o.DefaultFiltersDir,
originMarker: o.OriginMarker,
}, nil
}

Expand Down Expand Up @@ -311,25 +311,25 @@ func mapRoutes(r []*eskip.Route) map[string]*eskip.Route {
return m
}

func (c *Client) loadAndConvert() (*clusterState, []*eskip.Route, error) {
func (c *Client) loadAndConvert() ([]*eskip.Route, error) {
state, err := c.ClusterClient.fetchClusterState()
if err != nil {
return nil, nil, err
return nil, err
}

defaultFilters := c.fetchDefaultFilterConfigs()

ri, err := c.ingress.convert(state, defaultFilters)
if err != nil {
return nil, nil, err
return nil, err
}

rg, err := c.routeGroups.convert(state, defaultFilters)
if err != nil {
return nil, nil, err
return nil, err
}

return state, append(ri, rg...), nil
return append(ri, rg...), nil
}

func shuntRoute(r *eskip.Route) {
Expand Down Expand Up @@ -389,32 +389,9 @@ func (c *Client) hasReceivedTerm() bool {
return c.termReceived
}

func setOriginMarker(s *clusterState, r []*eskip.Route) []*eskip.Route {
or := &eskip.Route{
Id: "kube__originMarkers",
Predicates: []*eskip.Predicate{{
Name: primitive.NameFalse,
}},
BackendType: eskip.ShuntBackend,
}

for _, i := range s.ingresses {
or.Filters = append(
or.Filters,
builtin.NewOriginMarker(
ingressOriginName,
i.Metadata.Uid,
i.Metadata.Created,
),
)
}

return append(r, or)
}

func (c *Client) LoadAll() ([]*eskip.Route, error) {
log.Debug("loading all")
clusterState, r, err := c.loadAndConvert()
r, err := c.loadAndConvert()
if err != nil {
return nil, fmt.Errorf("failed to load cluster state: %w", err)
}
Expand All @@ -432,10 +409,6 @@ func (c *Client) LoadAll() ([]*eskip.Route, error) {
c.current = mapRoutes(r)
log.Debugf("all routes loaded and mapped")

if c.originMarker {
r = setOriginMarker(clusterState, r)
}

return r, nil
}

Expand All @@ -445,7 +418,7 @@ func (c *Client) LoadAll() ([]*eskip.Route, error) {
// TODO: implement a force reset after some time.
func (c *Client) LoadUpdate() ([]*eskip.Route, []string, error) {
log.Debugf("polling for updates")
clusterState, r, err := c.loadAndConvert()
r, err := c.loadAndConvert()
if err != nil {
log.Errorf("polling for updates failed: %v", err)
return nil, nil, err
Expand Down Expand Up @@ -489,10 +462,6 @@ func (c *Client) LoadUpdate() ([]*eskip.Route, []string, error) {
}
}

if c.originMarker && (len(updatedRoutes) > 0 || len(deletedIDs) > 0) {
updatedRoutes = setOriginMarker(clusterState, updatedRoutes)
}

c.current = next
return updatedRoutes, deletedIDs, nil
}
Expand Down
41 changes: 0 additions & 41 deletions dataclients/kubernetes/kube_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1162,47 +1162,6 @@ func TestIngress(t *testing.T) {
})
}

func TestOriginMarker(t *testing.T) {
api := newTestAPI(t, nil, &definitions.IngressList{})
defer api.Close()

api.services = testServices()
ii := testIngresses()
ti := time.Now().UTC()
var expected []string
for _, i := range ii {
n := i.Metadata.Name
expected = append(expected, n)
i.Metadata.Uid = n
i.Metadata.Created = ti
}
api.ingresses.Items = ii

dc, err := New(Options{KubernetesURL: api.server.URL, OriginMarker: true})
if err != nil {
t.Error(err)
}

defer dc.Close()

rr, err := dc.LoadAll()
require.NoError(t, err)

var actual []string
for _, r := range rr {
for _, f := range r.Filters {
if f.Name == builtin.OriginMarkerName {
require.Len(t, f.Args, 3)
actual = append(actual, f.Args[1].(string))
assert.Equal(t, ingressOriginName, f.Args[0])
assert.Equal(t, ti, f.Args[2])
}
}
}

assert.ElementsMatch(t, actual, expected)
}

func TestConvertPathRule(t *testing.T) {
api := newTestAPI(t, nil, &definitions.IngressList{})
defer api.Close()
Expand Down
107 changes: 0 additions & 107 deletions dataclients/kubernetes/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

"github.com/zalando/skipper/dataclients/kubernetes/definitions"
"github.com/zalando/skipper/eskip"
)

func TestUpdateOnlyChangedRoutes(t *testing.T) {
Expand Down Expand Up @@ -41,109 +40,3 @@ func TestUpdateOnlyChangedRoutes(t *testing.T) {
}
}
}

func TestOriginMarkerNotStored(t *testing.T) {
expectOriginMarker := func(r []*eskip.Route) {
for _, ri := range r {
for _, f := range ri.Filters {
if f.Name == "originMarker" {
return
}
}
}

t.Fatal("origin marker not found")
}

expectNoOriginMarker := func(r map[string]*eskip.Route) {
for _, ri := range r {
for _, f := range ri.Filters {
if f.Name == "originMarker" {
t.Fatal("unexpected origin marker found in stored routes")
}
}
}
}

api := newTestAPI(t,
&serviceList{
Items: []*service{
testService(
"namespace1",
"service1",
"1.2.3.4",
map[string]int{"port1": 8080},
),
}},
&definitions.IngressList{
Items: []*definitions.IngressItem{
testIngressSimple(
"namespace1",
"ingress1",
"service1",
definitions.BackendPort{Value: 8080},
testRule(
"example.org",
testPathRule("/", "service1", definitions.BackendPort{Value: 8080}),
),
),
},
},
)
defer api.Close()

k, err := New(Options{
KubernetesURL: api.server.URL,
ProvideHealthcheck: true,
OriginMarker: true,
})
if err != nil {
t.Fatal(err)
}

defer k.Close()

// receive initial routes
r, err := k.LoadAll()
if err != nil {
t.Fatal(err)
}

expectOriginMarker(r)
expectNoOriginMarker(k.current)

// check for an update, expect none
r, d, err := k.LoadUpdate()
if err != nil {
t.Fatal(err)
}

if len(r) != 0 || len(d) != 0 {
t.Fatal("unexpected route update received")
}

expectNoOriginMarker(k.current)

// make an update and receive it
api.ingresses.Items = append(
api.ingresses.Items,
testIngressSimple(
"namespace1",
"ingress2",
"service1",
definitions.BackendPort{Value: 8080},
testRule(
"api.example.org",
testPathRule("/v1", "service1", definitions.BackendPort{Value: 8080}),
),
),
)

r, _, err = k.LoadUpdate()
if err != nil {
t.Fatal(err)
}

expectOriginMarker(r)
expectNoOriginMarker(k.current)
}

0 comments on commit 9f8c62c

Please sign in to comment.