Skip to content

Commit

Permalink
Use capabilities structs in various types of server ads
Browse files Browse the repository at this point in the history 
This moves us away from using something like `ad.Writes` in favor of `ads.Caps.Writes`
by using our capabilities struct across the board where appropriate.

The work in this PR has been spread out over 3 releases to minimize potential impact
to backwards compatibility issues, with the 7.11 release targeted for final breaking
changes.
  • Loading branch information
@jhiemstrawisc
jhiemstrawisc committed Sep 24, 2024
1 parent 72438ca commit 41fe45f
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 58 deletions.
17 changes: 4 additions & 13 deletions director/advertise.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ func consolidateDupServerAd(newAd, existingAd server_structs.ServerAd) server_st
consolidatedAd.Caps.Writes = existingAd.Caps.Writes || newAd.Caps.Writes
consolidatedAd.Caps.Listings = existingAd.Caps.Listings || newAd.Caps.Listings

consolidatedAd.DirectReads = existingAd.DirectReads || newAd.DirectReads
consolidatedAd.Writes = existingAd.Writes || newAd.Writes
consolidatedAd.Listings = existingAd.Listings || newAd.Listings

return consolidatedAd
}

Expand All @@ -67,15 +63,11 @@ func parseServerAdFromTopology(server server_structs.TopoServer, serverType serv
// Explicitly set these to false for caches, because these caps don't really translate in that case
if serverAd.Type == server_structs.CacheType.String() {
serverAd.Caps = server_structs.Capabilities{}
serverAd.Writes = false
serverAd.Listings = false
serverAd.DirectReads = false
serverAd.Caps.Writes = false
serverAd.Caps.Listings = false
serverAd.Caps.DirectReads = false
serverAd.Caps.PublicReads = true
} else {
// Until we consolidate ServerAd capabilities with NamespaceAdV2 capabilities, we'll keep setting the top-level
// ServerAd capabilities. Eventually we should replace with the actual caps struct.
serverAd.Writes = caps.Writes
serverAd.Listings = caps.Listings
serverAd.DirectReads = caps.DirectReads
serverAd.Caps = caps
}

Expand Down Expand Up @@ -250,7 +242,6 @@ func AdvertiseOSDF(ctx context.Context) error {
}
nsAd := server_structs.NamespaceAdV2{
Path: ns.Path,
PublicRead: caps.PublicReads,
Caps: caps,
Generation: []server_structs.TokenGen{tGen},
Issuer: tokenIssuers,
Expand Down
33 changes: 14 additions & 19 deletions director/advertise_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,20 @@ var (

func TestConsolidateDupServerAd(t *testing.T) {
t.Run("union-capabilities", func(t *testing.T) {
existingAd := server_structs.ServerAd{Writes: false}
newAd := server_structs.ServerAd{Writes: true}
existingAd := server_structs.ServerAd{Caps: server_structs.Capabilities{Writes: false}}
newAd := server_structs.ServerAd{Caps: server_structs.Capabilities{Writes: true}}
get := consolidateDupServerAd(newAd, existingAd)
assert.True(t, get.Writes)
assert.True(t, get.Caps.Writes)

existingAd = server_structs.ServerAd{DirectReads: false}
newAd = server_structs.ServerAd{DirectReads: true}
existingAd = server_structs.ServerAd{Caps: server_structs.Capabilities{DirectReads: false}}
newAd = server_structs.ServerAd{Caps: server_structs.Capabilities{DirectReads: true}}
get = consolidateDupServerAd(newAd, existingAd)
assert.True(t, get.DirectReads)
assert.True(t, get.Caps.DirectReads)

existingAd = server_structs.ServerAd{Listings: false}
newAd = server_structs.ServerAd{Listings: true}
existingAd = server_structs.ServerAd{Caps: server_structs.Capabilities{Listings: false}}
newAd = server_structs.ServerAd{Caps: server_structs.Capabilities{Listings: true}}
get = consolidateDupServerAd(newAd, existingAd)
assert.True(t, get.Listings)
assert.True(t, get.Caps.Listings)

// All false
existingAd = server_structs.ServerAd{Caps: server_structs.Capabilities{}}
Expand Down Expand Up @@ -144,22 +144,19 @@ func TestParseServerAdFromTopology(t *testing.T) {
Writes: true,
Listings: true,
DirectReads: true,
PublicReads: true,
}
ad := parseServerAdFromTopology(server, server_structs.OriginType, caps)
assert.True(t, ad.Writes)
assert.True(t, ad.Caps.Writes)
assert.True(t, ad.Listings)
assert.True(t, ad.Caps.Listings)
assert.True(t, ad.DirectReads)
assert.True(t, ad.Caps.DirectReads)
assert.True(t, ad.Caps.PublicReads)

ad = parseServerAdFromTopology(server, server_structs.CacheType, caps)
assert.False(t, ad.Writes)
assert.False(t, ad.Caps.Writes)
assert.False(t, ad.Listings)
assert.False(t, ad.Caps.Listings)
assert.False(t, ad.DirectReads)
assert.False(t, ad.Caps.DirectReads)
assert.True(t, ad.Caps.PublicReads)
})

t.Run("test-invalid-url", func(t *testing.T) {
Expand Down Expand Up @@ -233,9 +230,7 @@ func TestAdvertiseOSDF(t *testing.T) {
assert.Equal(t, "https://cache2.com", cAds[0].URL.String())
// Check that various capabilities have survived until this point. Because these are from topology,
// origin and namespace caps should be the same
assert.True(t, oAds[0].Writes)
assert.True(t, oAds[0].Caps.Writes)
assert.True(t, oAds[0].Listings)
assert.True(t, oAds[0].Caps.Listings)
assert.False(t, oAds[0].Caps.PublicReads)
assert.True(t, nsAd.Caps.Writes)
Expand Down Expand Up @@ -273,8 +268,8 @@ func TestAdvertiseOSDF(t *testing.T) {
assert.Equal(t, server_structs.OriginType.String(), foundAd.Type)
assert.Len(t, foundAd.NamespaceAds, 3)
// This origin has at least one namespace enables the following capacity
assert.True(t, foundAd.DirectReads)
assert.True(t, foundAd.Writes)
assert.True(t, foundAd.Caps.DirectReads)
assert.True(t, foundAd.Caps.Writes)
assert.True(t, foundAd.Caps.PublicReads)
})

Expand Down
26 changes: 11 additions & 15 deletions director/director.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ func redirectToCache(ginCtx *gin.Context) {
if len(cacheAds) == 0 {
for _, originAd := range originAdsWObject {
// Find the first origin that enables direct reads as the fallback
if originAd.DirectReads {
if originAd.Caps.DirectReads {
cacheAds = append(cacheAds, originAd)
break
}
Expand Down Expand Up @@ -594,7 +594,7 @@ func redirectToOrigin(ginCtx *gin.Context) {
// Query Origins and check if the object exists
q = NewObjectStat()
qr := q.Query(context.Background(), reqPath, server_structs.OriginType, 1, 3,
withOriginAds(originAds), WithToken(reqParams.Get("authz")), withAuth(!namespaceAd.PublicRead))
withOriginAds(originAds), WithToken(reqParams.Get("authz")), withAuth(!namespaceAd.Caps.PublicReads))
log.Debugf("Stat result for %s: %s", reqPath, qr.String())

// For a successful response, we got a list of object URLs.
Expand Down Expand Up @@ -729,8 +729,8 @@ func redirectToOrigin(ginCtx *gin.Context) {
// If the namespace or the origin does not allow directory listings, then we should not advertise a collections-url.
// This is because the configuration of the origin/namespace should override the inclusion of "dirlisthost" for that origin.
// Listings is true by default so if it is ever set to false we should accept that config over the dirlisthost.
if namespaceAd.Caps.Listings && len(availableAds) > 0 && availableAds[0].Listings {
if !namespaceAd.PublicRead && availableAds[0].AuthURL != (url.URL{}) {
if namespaceAd.Caps.Listings && len(availableAds) > 0 && availableAds[0].Caps.Listings {
if !namespaceAd.Caps.PublicReads && availableAds[0].AuthURL != (url.URL{}) {
colUrl = availableAds[0].AuthURL.String()
} else {
colUrl = availableAds[0].URL.String()
Expand All @@ -743,8 +743,8 @@ func redirectToOrigin(ginCtx *gin.Context) {
// If we are doing a PROPFIND, check if origins enable dirlistings
if ginCtx.Request.Method == "PROPFIND" {
for idx, ad := range availableAds {
if ad.Listings && namespaceAd.Caps.Listings {
redirectURL = getRedirectURL(reqPath, availableAds[idx], !namespaceAd.PublicRead)
if ad.Caps.Listings && namespaceAd.Caps.Listings {
redirectURL = getRedirectURL(reqPath, availableAds[idx], !namespaceAd.Caps.PublicReads)
if brokerUrl := availableAds[idx].BrokerURL; brokerUrl.String() != "" {
ginCtx.Header("X-Pelican-Broker", brokerUrl.String())
}
Expand All @@ -765,8 +765,8 @@ func redirectToOrigin(ginCtx *gin.Context) {
// Check if we are doing a DirectRead and if it is allowed
if reqParams.Has(utils.QueryDirectRead.String()) {
for idx, originAd := range availableAds {
if originAd.DirectReads && namespaceAd.Caps.DirectReads {
redirectURL = getRedirectURL(reqPath, availableAds[idx], !namespaceAd.PublicRead)
if originAd.Caps.DirectReads && namespaceAd.Caps.DirectReads {
redirectURL = getRedirectURL(reqPath, availableAds[idx], !namespaceAd.Caps.PublicReads)
if brokerUrl := availableAds[idx].BrokerURL; brokerUrl.String() != "" {
ginCtx.Header("X-Pelican-Broker", brokerUrl.String())
}
Expand All @@ -788,8 +788,8 @@ func redirectToOrigin(ginCtx *gin.Context) {
// If we are doing a PUT, check to see if any origins are writeable
if ginCtx.Request.Method == "PUT" {
for idx, ad := range availableAds {
if ad.Writes {
redirectURL = getRedirectURL(reqPath, availableAds[idx], !namespaceAd.PublicRead)
if ad.Caps.Writes {
redirectURL = getRedirectURL(reqPath, availableAds[idx], !namespaceAd.Caps.PublicReads)
if brokerUrl := availableAds[idx].BrokerURL; brokerUrl.String() != "" {
ginCtx.Header("X-Pelican-Broker", brokerUrl.String())
}
Expand All @@ -803,7 +803,7 @@ func redirectToOrigin(ginCtx *gin.Context) {
})
return
} else { // Otherwise, we are doing a GET
redirectURL := getRedirectURL(reqPath, availableAds[0], !namespaceAd.PublicRead)
redirectURL := getRedirectURL(reqPath, availableAds[0], !namespaceAd.Caps.PublicReads)
if brokerUrl := availableAds[0].BrokerURL; brokerUrl.String() != "" {
ginCtx.Header("X-Pelican-Broker", brokerUrl.String())
}
Expand Down Expand Up @@ -1112,9 +1112,6 @@ func registerServeAd(engineCtx context.Context, ctx *gin.Context, sType server_s
BrokerURL: *brokerUrl,
Type: sType.String(),
Caps: adV2.Caps,
Writes: adV2.Caps.Writes,
DirectReads: adV2.Caps.DirectReads,
Listings: adV2.Caps.Listings,
IOLoad: 0.0, // Explicitly set to 0. The sort algorithm takes 0.0 as unknown load
}

Expand Down Expand Up @@ -1215,7 +1212,6 @@ func listNamespacesV1(ctx *gin.Context) {
func listNamespacesV2(ctx *gin.Context) {
namespacesAdsV2 := listNamespacesFromOrigins()
namespacesAdsV2 = append(namespacesAdsV2, server_structs.NamespaceAdV2{
PublicRead: true,
Caps: server_structs.Capabilities{
PublicReads: true,
Reads: true,
Expand Down
1 change: 0 additions & 1 deletion origin/advertise.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ func (server *OriginServer) CreateAdvertisement(name, originUrlStr, originWebUrl
// PublicReads implies reads
reads := export.Capabilities.PublicReads || export.Capabilities.Reads
nsAds = append(nsAds, server_structs.NamespaceAdV2{
PublicRead: export.Capabilities.PublicReads,
Caps: server_structs.Capabilities{
PublicReads: export.Capabilities.PublicReads,
Reads: reads,
Expand Down
11 changes: 2 additions & 9 deletions server_structs/director.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ type (
}

NamespaceAdV2 struct {
// TODO: Deprecate this top-level PublicRead field in favor of the Caps.PublicReads field.
// Should be done ~v7.10 series
PublicRead bool
Caps Capabilities // Namespace capabilities should be considered independently of the origin’s capabilities.
Caps Capabilities `json:"capabilities"` // Namespace capabilities should be considered independently of the origin’s capabilities.
Path string `json:"path"`
Generation []TokenGen `json:"token-generation"`
Issuer []TokenIssuer `json:"token-issuer"`
Expand Down Expand Up @@ -87,10 +84,7 @@ type (
Type string `json:"type"`
Latitude float64 `json:"latitude"`
Longitude float64 `json:"longitude"`
Caps Capabilities `json:"capabilities"` // TODO: Get rid of Writes, Listings, DirectReads in favor of Caps.Writes, Caps.Listings, Caps.DirectReads
Writes bool `json:"enable_write"`
Listings bool `json:"enable_listing"` // True if the origin allows directory listings
DirectReads bool `json:"enable_fallback_read"` // True if reads from the origin are permitted when no cache is available
Caps Capabilities `json:"capabilities"`
FromTopology bool `json:"from_topology"`
IOLoad float64 `json:"io_load"`
}
Expand Down Expand Up @@ -456,7 +450,6 @@ func ConvertNamespaceAdsV1ToV2(nsAdsV1 []NamespaceAdV1, oAd *OriginAdvertiseV1)
}

newNS := NamespaceAdV2{
PublicRead: caps.PublicReads,
Caps: caps,
Path: nsAd.Path,
}
Expand Down
1 change: 0 additions & 1 deletion server_structs/director_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func TestConversion(t *testing.T) {
}},
},
{
PublicRead: true,
Caps: Capabilities{
PublicReads: true,
Reads: true,
Expand Down

0 comments on commit 41fe45f

Please sign in to comment.