Skip to content

Commit

Permalink
Revert "Implement a TTL after which metadata/userdata expires"
Browse files Browse the repository at this point in the history
This reverts commit aeeed7a.
  • Loading branch information
ScottGarman committed Aug 16, 2023
1 parent 35a5121 commit 90ac0a6
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 117 deletions.
5 changes: 0 additions & 5 deletions cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import (
const (
serviceName = "metadata-service"

cacheTTLDefault = time.Hour

dbMaxRetriesDefault = 5
dbRetryMaxIntervalDefault = 3 * time.Second

Expand Down Expand Up @@ -115,9 +113,6 @@ func init() {
serveCmd.Flags().String("user-state-url", "", "An optional golang template string used to build a URL which instances can use for sending user state events. This template string will be evaluated against the instance metadata, and appended as a 'user_state_url' field on the metadata document served to instances. If no template string is specified, the 'user_state_url' field will not be added to the metadata document.")
viperBindFlag("metadata.user_state_url", serveCmd.Flags().Lookup("user-state-url"))

serveCmd.Flags().Duration("cache-ttl", cacheTTLDefault, "TTL, in seconds, to consider metadata and userdata valid for. Set to 0 to disable.")
viperBindFlag("cache_ttl", serveCmd.Flags().Lookup("cache-ttl"))

serveCmd.Flags().Duration("shutdown-grace-period", shutdownGracePeriod, "The grace period for requests to finish before forcibly exiting.")
viperBindFlag("shutdown_grace_period", serveCmd.Flags().Lookup("shutdown-grace-period"))
}
Expand Down
36 changes: 0 additions & 36 deletions pkg/api/v1/router_instance_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,6 @@ func (r *Router) instanceMetadataGet(c *gin.Context) {
return
}

// Treat metadata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if metadata != nil && cacheTTL != 0 && metadata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("Metadata for ", metadata.ID, " is older than our TTL, ignoring")
notFoundResponse(c)

return
}

if metadata != nil {
augmentedMetadata, err := addTemplateFields(metadata.Metadata, r.TemplateFields)
if err != nil {
Expand Down Expand Up @@ -149,15 +140,6 @@ func (r *Router) instanceMetadataExistsInternal(c *gin.Context) {
return
}

// Treat metadata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if metadata != nil && cacheTTL != 0 && metadata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("Metadata for ", metadata.ID, " is older than our TTL, ignoring (exists check)")
c.Status(http.StatusNotFound)

return
}

// HEAD request responses still set the Content-Length header to what it
// would be if we were returning the metadata
bytes, err := json.Marshal(metadata.Metadata)
Expand All @@ -183,15 +165,6 @@ func (r *Router) instanceUserdataGet(c *gin.Context) {
return
}

// Treat userdata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if userdata != nil && cacheTTL != 0 && userdata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("Userdata for ", userdata.ID, " is older than our TTL, ignoring")
notFoundResponse(c)

return
}

if userdata != nil {
c.String(http.StatusOK, string(userdata.Userdata.Bytes))
} else {
Expand Down Expand Up @@ -246,15 +219,6 @@ func (r *Router) instanceUserdataExistsInternal(c *gin.Context) {
return
}

// Treat userdata that's older then its TTL as non-existent
cacheTTL := viper.GetDuration("cache_ttl")
if userdata != nil && cacheTTL != 0 && userdata.UpdatedAt.Add(cacheTTL).Before(time.Now()) {
r.Logger.Sugar().Info("Userdata for ", userdata.ID, " is older than our TTL, ignoring (exists check)")
c.Status(http.StatusNotFound)

return
}

// HEAD request responses still set the Content-Length header to what it
// would be if we were returning the userdata
c.Writer.Header().Set("Content-Length", strconv.Itoa(len(userdata.Userdata.Bytes)))
Expand Down
38 changes: 0 additions & 38 deletions pkg/api/v1/router_instance_metadata_lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.hollow.sh/metadataservice/internal/lookup"
Expand Down Expand Up @@ -72,40 +70,4 @@ func TestGetMetadataLookupByIP(t *testing.T) {
assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Verify cache TTL settings are honored by setting the TTL to -1 second so we can avoid having to sleep
viper.Set("cache_ttl", -time.Second)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetMetadataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

if testcase.expectedStatus != http.StatusInternalServerError {
testcase.expectedStatus = http.StatusNotFound
}

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Setting cache_ttl to zero should disable the TTL behavior
viper.Set("cache_ttl", 0)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetMetadataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}
}
38 changes: 0 additions & 38 deletions pkg/api/v1/router_instance_userdata_lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"

"go.hollow.sh/metadataservice/internal/lookup"
Expand Down Expand Up @@ -72,40 +70,4 @@ func TestGetUserdataLookupByIP(t *testing.T) {
assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Verify cache TTL settings are honored by setting the TTL to -1 second so we can avoid having to sleep
viper.Set("cache_ttl", -time.Second)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetUserdataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

if testcase.expectedStatus == http.StatusOK {
testcase.expectedStatus = http.StatusNotFound
}

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}

// Setting cache_ttl to zero should disable the TTL behavior
viper.Set("cache_ttl", 0)

for _, testcase := range testCases {
t.Run(testcase.testName, func(t *testing.T) {
lookupClient.setResponse(testcase.instanceIP, testcase.lookupResponse)
w := httptest.NewRecorder()

req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, v1api.GetUserdataPath(), nil)
req.RemoteAddr = net.JoinHostPort(testcase.instanceIP, "")
router.ServeHTTP(w, req)

assert.Equal(t, testcase.expectedStatus, w.Code)
})
}
}

0 comments on commit 90ac0a6

Please sign in to comment.