Merge pull request #995 from ziyulin0/attribute

add attributes from reading configuration

config/cache.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"cloud.google.com/go/storage"
+
 	configpb "github.com/GoogleCloudPlatform/testgrid/pb/config"
 	"github.com/GoogleCloudPlatform/testgrid/util/gcs"
 )
@@ -39,6 +40,7 @@ const cacheRefreshInterval = 5 * time.Minute
 // Config holds a config proto and when it was fetched.
 type Config struct {
 	proto     *configpb.Configuration
+	attrs     *storage.ReaderObjectAttrs
 	lastFetch time.Time
 }
 
@@ -58,28 +60,29 @@ func InitCache() {
 // ReadGCS opens the config at path and unmarshals it into a Configuration proto.
 //
 // If it has been read recently, a cached version will be served.
-func ReadGCS(ctx context.Context, opener gcs.Opener, path gcs.Path) (*configpb.Configuration, error) {
+func ReadGCS(ctx context.Context, opener gcs.Opener, path gcs.Path) (*configpb.Configuration, *storage.ReaderObjectAttrs, error) {
 	cacheLock.Lock()
 	defer cacheLock.Unlock()
 	cfg, ok := cache[path.String()]
 	if ok && time.Since(cfg.lastFetch) < cacheRefreshInterval {
-		return cfg.proto, nil
+		return cfg.proto, cfg.attrs, nil
 	}
 
-	r, _, err := opener.Open(ctx, path)
+	r, attrs, err := opener.Open(ctx, path)
 	if err != nil {
-		return nil, fmt.Errorf("failed to open config: %w", err)
+		return nil, nil, fmt.Errorf("failed to open config: %w", err)
 	}
 	p, err := Unmarshal(r)
 	defer r.Close()
 	if err != nil {
-		return nil, fmt.Errorf("failed to unmarshal config: %w", err)
+		return nil, nil, fmt.Errorf("failed to unmarshal config: %w", err)
 	}
 	cache[path.String()] = Config{
 		proto:     p,
+		attrs:     attrs,
 		lastFetch: time.Now(),
 	}
-	return p, nil
+	return p, attrs, nil
 }
 
 // ReadPath reads the config from the specified local file path.
@@ -100,7 +103,8 @@ func Read(ctx context.Context, path string, client *storage.Client) (*configpb.C
 		if err != nil {
 			return nil, fmt.Errorf("bad path: %v", err)
 		}
-		return ReadGCS(ctx, gcs.NewClient(client), *gcsPath)
+		c, _, err := ReadGCS(ctx, gcs.NewClient(client), *gcsPath)
+		return c, err
 	}
 	return ReadPath(path)
 }

config/cache_test.go

@@ -23,6 +23,8 @@ import (
 
 	"cloud.google.com/go/storage"
 	"github.com/golang/protobuf/proto"
+	"github.com/google/go-cmp/cmp"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	configpb "github.com/GoogleCloudPlatform/testgrid/pb/config"
 	"github.com/GoogleCloudPlatform/testgrid/util/gcs"
@@ -62,7 +64,11 @@ func Test_ReadGCS(t *testing.T) {
 			name: "don't read if too recent",
 			currentCache: map[string]Config{
 				"gs://example": {
-					proto:     expectedConfig,
+					proto: expectedConfig,
+					attrs: &storage.ReaderObjectAttrs{
+						LastModified: now,
+						Generation:   1,
+					},
 					lastFetch: now,
 				},
 			},
@@ -96,21 +102,26 @@ func Test_ReadGCS(t *testing.T) {
 			client := fake.Client{
 				Opener: fake.Opener{},
 			}
+			expectedAttrs := &storage.ReaderObjectAttrs{
+				LastModified: test.remoteLastModified,
+				Generation:   test.remoteGeneration,
+			}
 
 			client.Opener[mustPath("gs://example")] = fake.Object{
-				Data: string(test.remoteData),
-				Attrs: &storage.ReaderObjectAttrs{
-					LastModified: test.remoteLastModified,
-					Generation:   test.remoteGeneration,
-				},
+				Data:  string(test.remoteData),
+				Attrs: expectedAttrs,
 			}
-			result, err := ReadGCS(context.Background(), &client, mustPath("gs://example"))
+			result, attrs, err := ReadGCS(context.Background(), &client, mustPath("gs://example"))
 			if err != nil {
 				t.Errorf("Unexpected error %v", err)
 			}
-			if !proto.Equal(expectedConfig, result) {
-				t.Errorf("Expected %v, got %v", expectedConfig, result)
+			if diff := cmp.Diff(expectedAttrs, attrs, protocmp.Transform()); diff != "" {
+				t.Errorf("Attributes differed (-want, +got): %s", diff)
 			}
+			if diff := cmp.Diff(expectedConfig, result, protocmp.Transform()); diff != "" {
+				t.Errorf("Config differed (-want, +got): %s ", diff)
+			}
+
 		})
 	}
 }

pkg/api/v1/config.go

@@ -62,7 +62,7 @@ func (s *Server) getConfig(ctx context.Context, scope string) (*configpb.Configu
 		return nil, errors.New("Scope not specified")
 	}
 
-	cfg, err := config.ReadGCS(ctx, s.Client, *configPath)
+	cfg, _, err := config.ReadGCS(ctx, s.Client, *configPath)
 	if err != nil {
 		// Only log an error if we set and use a default scope, but can't access it.
 		// Otherwise, invalid requests will write useless logs.

pkg/merger/merger.go

@@ -22,14 +22,14 @@ import (
 	"fmt"
 	"sync"
 
+	"github.com/golang/protobuf/proto"
+	"github.com/sirupsen/logrus"
+	"gopkg.in/yaml.v2"
+
 	"github.com/GoogleCloudPlatform/testgrid/config"
 	configpb "github.com/GoogleCloudPlatform/testgrid/pb/config"
 	"github.com/GoogleCloudPlatform/testgrid/util/gcs"
 	"github.com/GoogleCloudPlatform/testgrid/util/metrics"
-
-	"github.com/golang/protobuf/proto"
-	"github.com/sirupsen/logrus"
-	"gopkg.in/yaml.v2"
 )
 
 const componentName = "config-merger"
@@ -132,7 +132,7 @@ func MergeAndUpdate(ctx context.Context, client mergeClient, mets *Metrics, list
 		source := source
 		go func() {
 			defer wg.Done()
-			cfg, err := config.ReadGCS(ctx, client, *source.Path)
+			cfg, _, err := config.ReadGCS(ctx, client, *source.Path)
 			if err != nil {
 				// Log each fatal error, but it's okay to return any fatal error
 				logrus.WithError(err).WithFields(logrus.Fields{