Skip to content

Commit

Permalink
[agent-smith] Remove egress check feature
Browse files Browse the repository at this point in the history
  • Loading branch information
aledbf committed Apr 30, 2022
1 parent a5e608f commit 28fae44
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 331 deletions.
3 changes: 0 additions & 3 deletions components/ee/agent-smith/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
"blacklists": {
"$ref": "#/definitions/"
},
"egressTraffic": {
"$ref": "#/definitions/"
},
"enforcement": {
"$ref": "#/definitions/"
},
Expand Down
11 changes: 0 additions & 11 deletions components/ee/agent-smith/example-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,5 @@
}
]
}
},
"egressTraffic": {
"dt": "2m",
"excessive": {
"baseBudget": "300Mi",
"perDtThreshold": "100Mi"
},
"veryExcessive": {
"baseBudget": "2Gi",
"perDtThreshold": "250Mi"
}
}
}
110 changes: 15 additions & 95 deletions components/ee/agent-smith/pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,24 @@ import (
"context"
"fmt"
"net/url"
"os"
"sort"
"strings"
"sync"
"time"

"github.com/gitpod-io/gitpod/agent-smith/pkg/classifier"
"github.com/gitpod-io/gitpod/agent-smith/pkg/common"
"github.com/gitpod-io/gitpod/agent-smith/pkg/config"
"github.com/gitpod-io/gitpod/agent-smith/pkg/detector"
"github.com/gitpod-io/gitpod/common-go/log"
gitpod "github.com/gitpod-io/gitpod/gitpod-protocol"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/xerrors"

"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/utils/lru"

"github.com/gitpod-io/gitpod/agent-smith/pkg/classifier"
"github.com/gitpod-io/gitpod/agent-smith/pkg/common"
"github.com/gitpod-io/gitpod/agent-smith/pkg/config"
"github.com/gitpod-io/gitpod/agent-smith/pkg/detector"
"github.com/gitpod-io/gitpod/common-go/log"
gitpod "github.com/gitpod-io/gitpod/gitpod-protocol"
)

const (
Expand All @@ -42,9 +41,8 @@ type Smith struct {
Kubernetes kubernetes.Interface
metrics *metrics

egressTrafficCheckHandler func(pid int) (int64, error)
timeElapsedHandler func(t time.Time) time.Duration
notifiedInfringements *lru.Cache
timeElapsedHandler func(t time.Time) time.Duration
notifiedInfringements *lru.Cache

detector detector.ProcessDetector
classifier classifier.ProcessClassifier
Expand Down Expand Up @@ -112,10 +110,9 @@ func NewAgentSmith(cfg config.Config) (*Smith, error) {
res := &Smith{
EnforcementRules: map[string]config.EnforcementRules{
defaultRuleset: {
config.GradeKind(config.InfringementExec, common.SeverityBarely): config.PenaltyLimitCPU,
config.GradeKind(config.InfringementExec, common.SeverityAudit): config.PenaltyStopWorkspace,
config.GradeKind(config.InfringementExec, common.SeverityVery): config.PenaltyStopWorkspaceAndBlockUser,
config.GradeKind(config.InfringementExcessiveEgress, common.SeverityVery): config.PenaltyStopWorkspace,
config.GradeKind(config.InfringementExec, common.SeverityBarely): config.PenaltyLimitCPU,
config.GradeKind(config.InfringementExec, common.SeverityAudit): config.PenaltyStopWorkspace,
config.GradeKind(config.InfringementExec, common.SeverityVery): config.PenaltyStopWorkspaceAndBlockUser,
},
},
Config: cfg,
Expand All @@ -125,10 +122,9 @@ func NewAgentSmith(cfg config.Config) (*Smith, error) {
detector: detec,
classifier: class,

notifiedInfringements: lru.New(notificationCacheSize),
metrics: m,
egressTrafficCheckHandler: getEgressTraffic,
timeElapsedHandler: time.Since,
notifiedInfringements: lru.New(notificationCacheSize),
metrics: m,
timeElapsedHandler: time.Since,
}
if cfg.Enforcement.Default != nil {
if err := cfg.Enforcement.Default.Validate(); err != nil {
Expand Down Expand Up @@ -240,40 +236,6 @@ func (agent *Smith) Start(ctx context.Context, callback func(InfringingWorkspace
}()
}

egressTicker := time.NewTicker(2 * time.Minute)

go func() {
for {
<-egressTicker.C
wsMutex.Lock()
for pid, ws := range workspaces {
// check if the workspace is already stopped
fi, err := os.Stat(fmt.Sprintf("/proc/%d", pid))
if err != nil {
if os.IsNotExist(err) {
log.Debugf("deleting workspace with pid %d and workspaceId %s from workspaces", pid, ws.WorkspaceID)
delete(workspaces, pid)
} else {
log.WithError(err).WithFields(log.OWI(ws.OwnerID, ws.WorkspaceID, ws.InstanceID)).Error("could not stat workspace, stale entries will continue to exist in workspaces state")
}
continue
}
infringement, err := agent.checkEgressTrafficCallback(pid, fi.ModTime())
if err != nil {
log.WithError(err).WithFields(log.OWI(ws.OwnerID, ws.WorkspaceID, ws.InstanceID)).Error("error calling checkEgressTrafficCallback")
continue
}
if infringement != nil {
log.WithFields(log.OWI(ws.OwnerID, ws.WorkspaceID, ws.InstanceID)).Info("found egress infringing workspace")
// monitor metric here, as we are not penalizing workspaces yet
agent.metrics.egressViolations.WithLabelValues(string(infringement.Kind)).Inc()
}

}
wsMutex.Unlock()
}
}()

defer log.Info("agent smith main loop ended")

// We want to fill the classifier in a Go routine seaparete from using the classification
Expand Down Expand Up @@ -429,45 +391,3 @@ func (agent *Smith) Collect(m chan<- prometheus.Metric) {
agent.classifier.Collect(m)
agent.detector.Collect(m)
}

func (agent *Smith) checkEgressTrafficCallback(pid int, pidCreationTime time.Time) (*Infringement, error) {
if agent.Config.EgressTraffic == nil {
return nil, nil
}

podLifetime := agent.timeElapsedHandler(pidCreationTime)
resp, err := agent.egressTrafficCheckHandler(pid)
if err != nil {
return nil, err
}

log.Debugf("total egress bytes %d", resp)
if resp <= 0 {
log.WithField("total egress bytes", resp).Warn("GetEgressTraffic returned <= 0 value")
return nil, nil
}

type level struct {
V config.GradedInfringementKind
T *config.PerLevelEgressTraffic
}
levels := make([]level, 0, 2)
if agent.Config.EgressTraffic.VeryExcessiveLevel != nil {
levels = append(levels, level{V: config.GradeKind(config.InfringementExcessiveEgress, common.SeverityVery), T: agent.Config.EgressTraffic.VeryExcessiveLevel})
}
if agent.Config.EgressTraffic.ExcessiveLevel != nil {
levels = append(levels, level{V: config.GradeKind(config.InfringementExcessiveEgress, common.SeverityAudit), T: agent.Config.EgressTraffic.ExcessiveLevel})
}

dt := int64(podLifetime / time.Duration(agent.Config.EgressTraffic.WindowDuration))
for _, lvl := range levels {
allowance := dt*lvl.T.Threshold.Value() + lvl.T.BaseBudget.Value()
excess := resp - allowance

if excess > 0 {
return &Infringement{Description: fmt.Sprintf("egress traffic is %.3f megabytes over limit", float64(excess)/(1024.0*1024.0)), Kind: lvl.V}, nil
}
}

return nil, nil
}
153 changes: 7 additions & 146 deletions components/ee/agent-smith/pkg/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,8 @@
package agent

import (
"encoding/json"
"fmt"
"io/ioutil"
"path"
"reflect"
"sort"
"testing"
"time"

"github.com/gitpod-io/gitpod/agent-smith/pkg/common"
"github.com/gitpod-io/gitpod/agent-smith/pkg/config"
Expand Down Expand Up @@ -62,20 +56,19 @@ func TestGetPenalty(t *testing.T) {

func TestFindEnforcementRules(t *testing.T) {
ra := config.EnforcementRules{config.GradeKind(config.InfringementExec, common.SeverityAudit): config.PenaltyLimitCPU}
rb := config.EnforcementRules{config.GradeKind(config.InfringementExcessiveEgress, common.SeverityAudit): config.PenaltyLimitCPU}
tests := []struct {
Desc string
Rules map[string]config.EnforcementRules
RemoteURL string
Expectation config.EnforcementRules
}{
{"direct match", map[string]config.EnforcementRules{"foo": ra, "bar": rb}, "foo", ra},
{"no match", map[string]config.EnforcementRules{"foo*": ra, "bar": rb}, "not found", nil},
{"star", map[string]config.EnforcementRules{"foo*": ra, "bar": rb}, "foobar", ra},
{"prefix match", map[string]config.EnforcementRules{"*foo": ra, "bar": rb}, "hello/foo", ra},
{"suffix match", map[string]config.EnforcementRules{"foo*": ra, "bar": rb}, "foobar", ra},
{"case-insensitive match", map[string]config.EnforcementRules{"foo*": ra, "bar": rb}, "Foobar", ra},
{"submatch", map[string]config.EnforcementRules{"*foo*": ra, "bar": rb}, "hello/foo/bar", ra},
{"direct match", map[string]config.EnforcementRules{"foo": ra}, "foo", ra},
{"no match", map[string]config.EnforcementRules{"foo*": ra}, "not found", nil},
{"star", map[string]config.EnforcementRules{"foo*": ra}, "foobar", ra},
{"prefix match", map[string]config.EnforcementRules{"*foo": ra}, "hello/foo", ra},
{"suffix match", map[string]config.EnforcementRules{"foo*": ra}, "foobar", ra},
{"case-insensitive match", map[string]config.EnforcementRules{"foo*": ra}, "Foobar", ra},
{"submatch", map[string]config.EnforcementRules{"*foo*": ra}, "hello/foo/bar", ra},
}

for _, test := range tests {
Expand All @@ -101,135 +94,3 @@ func BenchmarkFindEnforcementRules(b *testing.B) {
findEnforcementRules(rules, "foobar")
}
}

func TestCeckEgressTrafficCallback(t *testing.T) {
simpleTime, _ := time.Parse(time.RFC3339, "2021-07-05T15:16:17+02:00")

type args struct {
pid int
pidCreationTime time.Time
}
tests := map[string]struct {
args args
want *Infringement
egressTrafficCheckHandler func(pid int) (int64, error)
timeElapsedHandler func(t time.Time) time.Duration
wantErr bool
}{
"no_infringement": {
args: args{
pid: 1234,
pidCreationTime: simpleTime,
},
want: nil,
egressTrafficCheckHandler: func(pid int) (int64, error) {
return 2000000, nil
},
timeElapsedHandler: func(t time.Time) time.Duration {
d, _ := time.ParseDuration("1m")
return d
},
wantErr: false,
},
"zero_egress": {
args: args{
pid: 1234,
pidCreationTime: simpleTime,
},
want: nil,
egressTrafficCheckHandler: func(pid int) (int64, error) {
return 0, nil
},
timeElapsedHandler: func(t time.Time) time.Duration {
d, _ := time.ParseDuration("1m")
return d
},
wantErr: false,
},
"excessive_egress": {
args: args{
pid: 1234,
pidCreationTime: simpleTime,
},
want: &Infringement{
Kind: config.GradedInfringementKind(config.InfringementExcessiveEgress),
Description: "egress traffic is 12.805 megabytes over limit",
},
egressTrafficCheckHandler: func(pid int) (int64, error) {
return 328000000, nil
},
timeElapsedHandler: func(t time.Time) time.Duration {
d, _ := time.ParseDuration("1m")
return d
},
wantErr: false,
},
"very_excessive_egress_simple": {
args: args{
pid: 1234,
pidCreationTime: simpleTime,
},
want: &Infringement{
Kind: config.GradeKind(config.InfringementExcessiveEgress, common.SeverityVery),
Description: "egress traffic is 188686.863 megabytes over limit",
},
egressTrafficCheckHandler: func(pid int) (int64, error) {
return 200000000000, nil
},
timeElapsedHandler: func(t time.Time) time.Duration {
d, _ := time.ParseDuration("1s")
return d
},
wantErr: false,
},
"very_excessive_egress": {
args: args{
pid: 1234,
pidCreationTime: simpleTime,
},
want: &Infringement{
Kind: config.GradeKind(config.InfringementExcessiveEgress, common.SeverityVery),
Description: "egress traffic is 188686.863 megabytes over limit",
},
egressTrafficCheckHandler: func(pid int) (int64, error) {
return 200000000000, nil
},
timeElapsedHandler: func(t time.Time) time.Duration {
d, _ := time.ParseDuration("1m")
return d
},
wantErr: false,
},
}

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
fc, err := ioutil.ReadFile(path.Join("testdata", fmt.Sprintf("agent_check_egress_%s.golden", name)))
if err != nil {
t.Errorf("cannot read config: %v", err)
return
}
var cfg config.Config
err = json.Unmarshal(fc, &cfg)
if err != nil {
t.Errorf("cannot unmarshal config: %v", err)
return
}
agent, err := NewAgentSmith(cfg)
if err != nil {
t.Errorf("cannot create test agent smith from config: %v", err)
return
}
agent.egressTrafficCheckHandler = tt.egressTrafficCheckHandler
agent.timeElapsedHandler = tt.timeElapsedHandler
got, err := agent.checkEgressTrafficCallback(tt.args.pid, tt.args.pidCreationTime)
if (err != nil) != tt.wantErr {
t.Errorf("Smith.checkEgressTrafficCallback() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Smith.checkEgressTrafficCallback() = %s", cmp.Diff(got, tt.want))
}
})
}
}

This file was deleted.

This file was deleted.

Loading

0 comments on commit 28fae44

Please sign in to comment.