diff --git a/client/nginx.go b/client/nginx.go index 47c2fb3e..4317ad95 100644 --- a/client/nginx.go +++ b/client/nginx.go @@ -808,6 +808,7 @@ func (client *NginxClient) DeleteHTTPServer(ctx context.Context, upstream string // Servers that are in the slice, but don't exist in NGINX will be added to NGINX. // Servers that aren't in the slice, but exist in NGINX, will be removed from NGINX. // Servers that are in the slice and exist in NGINX, but have different parameters, will be updated. +// The client will attempt to update all servers, returning all the errors that occurred. func (client *NginxClient) UpdateHTTPServers(ctx context.Context, upstream string, servers []UpstreamServer) (added []UpstreamServer, deleted []UpstreamServer, updated []UpstreamServer, err error) { serversInNginx, err := client.GetHTTPServers(ctx, upstream) if err != nil { @@ -824,27 +825,37 @@ func (client *NginxClient) UpdateHTTPServers(ctx context.Context, upstream strin toAdd, toDelete, toUpdate := determineUpdates(formattedServers, serversInNginx) for _, server := range toAdd { - err := client.AddHTTPServer(ctx, upstream, server) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to update servers of %v upstream: %w", upstream, err) + addErr := client.AddHTTPServer(ctx, upstream, server) + if addErr != nil { + err = errors.Join(err, addErr) + continue } + added = append(added, server) } for _, server := range toDelete { - err := client.DeleteHTTPServer(ctx, upstream, server.Server) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to update servers of %v upstream: %w", upstream, err) + deleteErr := client.DeleteHTTPServer(ctx, upstream, server.Server) + if deleteErr != nil { + err = errors.Join(err, deleteErr) + continue } + deleted = append(deleted, server) } for _, server := range toUpdate { - err := client.UpdateHTTPServer(ctx, upstream, server) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to update servers of %v upstream: %w", upstream, err) + updateErr := client.UpdateHTTPServer(ctx, upstream, server) + if updateErr != nil { + err = errors.Join(err, updateErr) + continue } + updated = append(updated, server) + } + + if err != nil { + err = fmt.Errorf("failed to update servers of %s upstream: %w", upstream, err) } - return toAdd, toDelete, toUpdate, nil + return added, deleted, updated, err } // haveSameParameters checks if a given server has the same parameters as a server already present in NGINX. Order matters. @@ -1108,6 +1119,7 @@ func (client *NginxClient) DeleteStreamServer(ctx context.Context, upstream stri // Servers that are in the slice, but don't exist in NGINX will be added to NGINX. // Servers that aren't in the slice, but exist in NGINX, will be removed from NGINX. // Servers that are in the slice and exist in NGINX, but have different parameters, will be updated. +// The client will attempt to update all servers, returning all the errors that occurred. func (client *NginxClient) UpdateStreamServers(ctx context.Context, upstream string, servers []StreamUpstreamServer) (added []StreamUpstreamServer, deleted []StreamUpstreamServer, updated []StreamUpstreamServer, err error) { serversInNginx, err := client.GetStreamServers(ctx, upstream) if err != nil { @@ -1123,27 +1135,37 @@ func (client *NginxClient) UpdateStreamServers(ctx context.Context, upstream str toAdd, toDelete, toUpdate := determineStreamUpdates(formattedServers, serversInNginx) for _, server := range toAdd { - err := client.AddStreamServer(ctx, upstream, server) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to update stream servers of %v upstream: %w", upstream, err) + addErr := client.AddStreamServer(ctx, upstream, server) + if addErr != nil { + err = errors.Join(err, addErr) + continue } + added = append(added, server) } for _, server := range toDelete { - err := client.DeleteStreamServer(ctx, upstream, server.Server) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to update stream servers of %v upstream: %w", upstream, err) + deleteErr := client.DeleteStreamServer(ctx, upstream, server.Server) + if deleteErr != nil { + err = errors.Join(err, deleteErr) + continue } + deleted = append(deleted, server) } for _, server := range toUpdate { - err := client.UpdateStreamServer(ctx, upstream, server) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to update stream servers of %v upstream: %w", upstream, err) + updateErr := client.UpdateStreamServer(ctx, upstream, server) + if updateErr != nil { + err = errors.Join(err, updateErr) + continue } + updated = append(updated, server) + } + + if err != nil { + err = fmt.Errorf("failed to update stream servers of %s upstream: %w", upstream, err) } - return toAdd, toDelete, toUpdate, nil + return added, deleted, updated, err } func (client *NginxClient) getIDOfStreamServer(ctx context.Context, upstream string, name string) (int, error) { diff --git a/tests/client_test.go b/tests/client_test.go index 1217f5ad..5ace142c 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -304,6 +304,67 @@ func TestStreamUpstreamServer(t *testing.T) { } } +func TestUpdateStreamUpstreamServersWithError(t *testing.T) { + t.Parallel() + c, err := client.NewNginxClient(helpers.GetAPIEndpoint()) + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + maxFails := 64 + weight := 10 + maxConns := 321 + backup := true + down := true + + serversToUpdate := []client.StreamUpstreamServer{ + { + ID: -1, + Server: "127.0.0.1:3000", + }, + { + Server: "127.0.0.1:5000", + MaxConns: &maxConns, + MaxFails: &maxFails, + FailTimeout: "21s", + SlowStart: "12s", + Weight: &weight, + Backup: &backup, + Down: &down, + }, + } + ctx := context.Background() + + added, _, _, err := c.UpdateStreamServers(ctx, streamUpstream, serversToUpdate) + if err == nil { + t.Fatalf("Expected the client to return an error for adding a server with ID -1") + } + + if len(added) != 1 { + t.Fatalf("Expected one server to be added, instead %d were added", len(added)) + } + + servers, err := c.GetStreamServers(ctx, streamUpstream) + if err != nil { + t.Fatalf("Error getting stream servers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(serversToUpdate[1], servers[0]) { + t.Errorf("Expected: %v Got: %v", serversToUpdate[1], servers[0]) + } + + // remove stream upstream servers + _, _, _, err = c.UpdateStreamServers(ctx, streamUpstream, []client.StreamUpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + //nolint:paralleltest func TestClient(t *testing.T) { c, err := client.NewNginxClient(helpers.GetAPIEndpoint()) @@ -577,6 +638,68 @@ func TestUpstreamServer(t *testing.T) { } } +//nolint:paralleltest +func TestUpdateUpstreamServersWithError(t *testing.T) { + c, err := client.NewNginxClient(helpers.GetAPIEndpoint()) + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + maxFails := 64 + weight := 10 + maxConns := 321 + backup := true + down := true + + serversToUpdate := []client.UpstreamServer{ + { + ID: -1, + Server: "127.0.0.1:3000", + }, + { + Server: "127.0.0.1:2000", + MaxConns: &maxConns, + MaxFails: &maxFails, + FailTimeout: "21s", + SlowStart: "12s", + Weight: &weight, + Route: "test", + Backup: &backup, + Down: &down, + }, + } + + ctx := context.Background() + added, _, _, err := c.UpdateHTTPServers(ctx, upstream, serversToUpdate) + if err == nil { + t.Fatal("Expected the client to return an error for adding a server with ID -1") + } + + if len(added) != 1 { + t.Fatalf("Expected one server to be added, instead %d were added", len(added)) + } + + servers, err := c.GetHTTPServers(ctx, upstream) + if err != nil { + t.Fatalf("Error getting HTTPServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(serversToUpdate[1], servers[0]) { + t.Errorf("Expected: %v Got: %v", serversToUpdate[1], servers[0]) + } + + // remove upstream servers + _, _, _, err = c.UpdateHTTPServers(ctx, upstream, []client.UpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + //nolint:paralleltest func TestStats(t *testing.T) { c, err := client.NewNginxClient(helpers.GetAPIEndpoint())