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

[agent-smith] Remove egress check feature #9689

Merged
merged 2 commits into from
May 4, 2022
Merged

[agent-smith] Remove egress check feature #9689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31248ed
[agent-smith] Remove egress check feature
aledbf Apr 30, 2022
dad3890
Add missing go dependencies
aledbf May 2, 2022
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
3 changes: 0 additions & 3 deletions components/ee/agent-smith/config-schema.json
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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))
}
})
}
}
24 changes: 0 additions & 24 deletions components/ee/agent-smith/pkg/agent/egress.go
View file
Open in desktop

This file was deleted.

Loading