-
Notifications
You must be signed in to change notification settings - Fork 2
feat(control-loop/ensure-networks) Add notion of control loop, ensure network endpoints every 30 minutes #329
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package sand | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "net/http" | ||
|
|
||
| "github.com/pkg/errors" | ||
|
|
||
| "github.com/Scalingo/sand/api/httpresp" | ||
| ) | ||
|
|
||
| func (c *client) NodeEnsureNetworkEndpoints(ctx context.Context) error { | ||
| req, err := http.NewRequest("POST", fmt.Sprintf("%s/node/ensure-network-endpoints", c.url), nil) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to create HTTP request") | ||
| } | ||
| req = req.WithContext(ctx) | ||
| res, err := c.httpClient.Do(req) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to execute POST /node/ensure-network-endpoints") | ||
| } | ||
| defer res.Body.Close() | ||
|
|
||
| if res.StatusCode != http.StatusNoContent { | ||
| var reserr httpresp.Error | ||
| err := json.NewDecoder(res.Body).Decode(&reserr) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to decode JSON in errors response: %s", res.Status) | ||
| } | ||
|
|
||
| return reserr | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/urfave/cli" | ||
| ) | ||
|
|
||
| func (a *App) NodeEnsureNetworkEndpoints(c *cli.Context) error { | ||
| client, err := a.sandClient(c) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = client.NodeEnsureNetworkEndpoints(context.Background()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fmt.Println("Network endpoints ensure triggered") | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,18 +8,17 @@ import ( | |
| "path/filepath" | ||
|
|
||
| "github.com/moby/moby/pkg/reexec" | ||
| "github.com/pkg/errors" | ||
| "github.com/sirupsen/logrus" | ||
|
|
||
| "github.com/Scalingo/go-etcd-lock/v5/lock" | ||
| "github.com/Scalingo/go-handlers" | ||
| dockeripam "github.com/Scalingo/go-plugins-helpers/ipam" | ||
| dockernetwork "github.com/Scalingo/go-plugins-helpers/network" | ||
| dockersdk "github.com/Scalingo/go-plugins-helpers/sdk" | ||
| "github.com/Scalingo/go-utils/cronsetup" | ||
| "github.com/Scalingo/go-utils/graceful" | ||
| "github.com/Scalingo/go-utils/logger" | ||
| "github.com/Scalingo/go-utils/logger/plugins/rollbarplugin" | ||
| "github.com/Scalingo/sand/api/params" | ||
| "github.com/Scalingo/sand/api/types" | ||
| "github.com/Scalingo/sand/config" | ||
| "github.com/Scalingo/sand/endpoint" | ||
|
|
@@ -29,6 +28,7 @@ import ( | |
| "github.com/Scalingo/sand/network" | ||
| "github.com/Scalingo/sand/network/netmanager" | ||
| "github.com/Scalingo/sand/network/overlay" | ||
| "github.com/Scalingo/sand/node" | ||
| "github.com/Scalingo/sand/store" | ||
| apptls "github.com/Scalingo/sand/utils/tls" | ||
| "github.com/Scalingo/sand/web" | ||
|
|
@@ -83,11 +83,17 @@ func main() { | |
| endpointRepository := endpoint.NewRepository(c, dataStore, managers) | ||
| networkRepository := network.NewRepository(c, dataStore, managers) | ||
|
|
||
| err = ensureNetworks(ctx, c, networkRepository, endpointRepository) | ||
| err = node.EnsureNetworkEndpoints(ctx, c, networkRepository, endpointRepository) | ||
| if err != nil { | ||
| log.WithError(err).Error("fail to ensure existing networks") | ||
| os.Exit(-1) | ||
| } | ||
| stopEndpointEnsureCron, err := setupEndpointEnsureCron(ctx, c, networkRepository, endpointRepository) | ||
| if err != nil { | ||
| log.WithError(err).Error("fail to setup endpoint ensure cron") | ||
| os.Exit(-1) | ||
| } | ||
| defer stopEndpointEnsureCron() | ||
|
|
||
| vctrl := web.NewVersionController(c) | ||
| nctrl := web.NewNetworksController(c, networkRepository, endpointRepository, ipAllocator) | ||
|
|
@@ -96,6 +102,7 @@ func main() { | |
| sandRouter := handlers.NewRouter(log) | ||
| sandRouter.Use(handlers.ErrorMiddleware) | ||
| sandRouter.HandleFunc("/version", vctrl.Show).Methods("GET") | ||
| sandRouter.HandleFunc("/node/ensure-network-endpoints", nctrl.EnsureNetworkEndpoints).Methods("POST") | ||
| sandRouter.HandleFunc("/networks", nctrl.List).Methods("GET") | ||
| sandRouter.HandleFunc("/networks", nctrl.Create).Methods("POST") | ||
| sandRouter.HandleFunc("/networks/{id}", nctrl.Show).Methods("GET") | ||
|
|
@@ -175,69 +182,17 @@ func main() { | |
| log.Info("All APIs stopped, shutting down..") | ||
| } | ||
|
|
||
| func ensureNetworks(ctx context.Context, c *config.Config, repo network.Repository, erepo endpoint.Repository) error { | ||
| log := logger.Get(ctx) | ||
| ctx = logger.ToCtx(ctx, log) | ||
|
|
||
| log.Info("Ensure networks on node") | ||
|
|
||
| endpoints, err := erepo.List(ctx, map[string]string{"hostname": c.GetPeerHostname()}) | ||
| if err == store.ErrNotFound { | ||
| return nil | ||
| } | ||
| if err != nil { | ||
| return errors.Wrapf(err, "fail to list endpoints of %v", c.GetPeerHostname()) | ||
| } | ||
|
|
||
| for _, endpoint := range endpoints { | ||
| log = log.WithField("endpoint_id", endpoint.ID) | ||
| ctx = logger.ToCtx(ctx, log) | ||
| if !endpoint.Active { | ||
| log.Debug("skip inactive endpoint") | ||
| continue | ||
| } | ||
| log = log.WithFields(logrus.Fields{ | ||
| "network_id": endpoint.NetworkID, | ||
| "endpoint_id": endpoint.ID, | ||
| "endpoint_netns_path": endpoint.TargetNetnsPath, | ||
| }) | ||
| log.Info("restoring endpoint") | ||
|
|
||
| network, ok, err := repo.Exists(ctx, endpoint.NetworkID) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "fail to get network") | ||
| } | ||
| if !ok { | ||
| log.WithError(errors.Errorf("network not found for %v", endpoint)) | ||
| continue | ||
| } | ||
|
|
||
| log.Info("ensuring network") | ||
| err = repo.Ensure(ctx, network) | ||
| if err != nil { | ||
| log.WithError(err).Error("fail to ensure network") | ||
| continue | ||
| } | ||
|
|
||
| endpoint, err = erepo.Activate(ctx, network, endpoint, params.EndpointActivate{ | ||
| NSHandlePath: endpoint.TargetNetnsPath, | ||
| SetAddr: true, | ||
| MoveVeth: true, | ||
| }) | ||
| if err != nil { | ||
| // if we can't activate the endpoint because the netns path doesn't exist anymore, we | ||
| // just deactivate it. Otherwise we raise an error. | ||
| if os.IsNotExist(errors.Cause(err)) { | ||
| endpoint, err = erepo.Deactivate(ctx, network, endpoint) | ||
| if err != nil { | ||
| log.WithError(err).Error("fail to deactivate endpoint") | ||
| continue | ||
| } | ||
| } else { | ||
| log.WithError(err).Error("fail to ensure endpoint") | ||
| continue | ||
| } | ||
| } | ||
| } | ||
| return nil | ||
| func setupEndpointEnsureCron(ctx context.Context, c *config.Config, repo network.Repository, erepo endpoint.Repository) (func(), error) { | ||
| return cronsetup.Setup(ctx, cronsetup.SetupOpts{ | ||
| WithoutTelemetry: true, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: is there a reason why you remove the telemtry?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it wasn't installed yet in sand, this will change with my next PR |
||
| Jobs: []cronsetup.Job{ | ||
| { | ||
| Name: "ensure network endpoints", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: is it safe to have a name with spaces in it?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, poor review from my side |
||
| Rhythm: "@every " + c.EndpointEnsureInterval.String(), | ||
| Func: func(ctx context.Context) error { | ||
| return node.EnsureNetworkEndpoints(ctx, c, repo, erepo) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: wrap the error
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I could wrap it, but it doesn't bring a lot of value at this level I'd say, but ok |
||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,88 @@ | ||||||||||||||||||||
| package node | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import ( | ||||||||||||||||||||
| "context" | ||||||||||||||||||||
| "os" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| "github.com/pkg/errors" | ||||||||||||||||||||
| "github.com/sirupsen/logrus" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| "github.com/Scalingo/go-utils/logger" | ||||||||||||||||||||
| "github.com/Scalingo/sand/api/params" | ||||||||||||||||||||
| "github.com/Scalingo/sand/config" | ||||||||||||||||||||
| "github.com/Scalingo/sand/endpoint" | ||||||||||||||||||||
| "github.com/Scalingo/sand/network" | ||||||||||||||||||||
| "github.com/Scalingo/sand/store" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| func EnsureNetworkEndpoints(ctx context.Context, c *config.Config, repo network.Repository, erepo endpoint.Repository) error { | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I understand |
||||||||||||||||||||
| log := logger.Get(ctx) | ||||||||||||||||||||
| ctx = logger.ToCtx(ctx, log) | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| log.Info("Ensure networks on node") | ||||||||||||||||||||
|
|
||||||||||||||||||||
| endpoints, err := erepo.List(ctx, map[string]string{"hostname": c.GetPeerHostname()}) | ||||||||||||||||||||
| if errors.Cause(err) == store.ErrNotFound { | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: should we already switch to |
||||||||||||||||||||
| return nil | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||
| return errors.Wrapf(err, "failed to list endpoints of %v", c.GetPeerHostname()) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| for _, endpoint := range endpoints { | ||||||||||||||||||||
| endpointLog := log.WithFields(logrus.Fields{ | ||||||||||||||||||||
| "endpoint_id": endpoint.ID, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| endpointCtx := logger.ToCtx(ctx, endpointLog) | ||||||||||||||||||||
|
Comment on lines
+33
to
+36
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| if !endpoint.Active { | ||||||||||||||||||||
| endpointLog.Debug("skip inactive endpoint") | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| endpointLog = endpointLog.WithFields(logrus.Fields{ | ||||||||||||||||||||
| "network_id": endpoint.NetworkID, | ||||||||||||||||||||
| "endpoint_netns_path": endpoint.TargetNetnsPath, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| endpointCtx = logger.ToCtx(ctx, endpointLog) | ||||||||||||||||||||
|
Comment on lines
+43
to
+47
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
| endpointLog.Info("restoring endpoint") | ||||||||||||||||||||
|
|
||||||||||||||||||||
| network, ok, err := repo.Exists(endpointCtx, endpoint.NetworkID) | ||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||
| return errors.Wrapf(err, "failed to get network") | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if !ok { | ||||||||||||||||||||
| endpointLog.WithError(errors.Errorf("network not found for %v", endpoint)).Error("skip endpoint") | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| endpointLog.Info("ensuring network") | ||||||||||||||||||||
| err = repo.Ensure(endpointCtx, network) | ||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||
| endpointLog.WithError(err).Error("failed to ensure network") | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| _, err = erepo.Activate(endpointCtx, network, endpoint, params.EndpointActivate{ | ||||||||||||||||||||
| NSHandlePath: endpoint.TargetNetnsPath, | ||||||||||||||||||||
| SetAddr: true, | ||||||||||||||||||||
| MoveVeth: true, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||
| // If the netns path no longer exists, deactivate the endpoint. Other | ||||||||||||||||||||
| // activation errors should not prevent the remaining endpoints from being ensured. | ||||||||||||||||||||
| if os.IsNotExist(errors.Cause(err)) { | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep identified an issue in your code: A wrapped file-not-found error from More details about this
A plausible abuse case is: 1) an attacker causes To resolve this comment: ✨ Commit fix suggestion
Suggested change
View step-by-step instructions
💬 Ignore this findingReply with Semgrep commands to ignore this finding.
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by os-error-is-not-exist. You can view more details about this finding in the Semgrep AppSec Platform. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I second this |
||||||||||||||||||||
| _, err = erepo.Deactivate(endpointCtx, network, endpoint) | ||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||
| endpointLog.WithError(err).Error("failed to deactivate endpoint") | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| endpointLog.WithError(err).Error("failed to ensure endpoint") | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return nil | ||||||||||||||||||||
| } | ||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: wrap the errors