Add immediate inline tiering support (#13298)

cmd/bucket-lifecycle.go

@@ -220,16 +220,31 @@ var errInvalidStorageClass = errors.New("invalid storage class")
 
 func validateTransitionTier(lc *lifecycle.Lifecycle) error {
 	for _, rule := range lc.Rules {
-		if rule.Transition.StorageClass == "" {
+		if rule.Transition.StorageClass != "" {
-			continue
+			if valid := globalTierConfigMgr.IsTierValid(rule.Transition.StorageClass); !valid {
+				return errInvalidStorageClass
+			}
 		}
-		if valid := globalTierConfigMgr.IsTierValid(rule.Transition.StorageClass); !valid {
+		if rule.NoncurrentVersionTransition.StorageClass != "" {
-			return errInvalidStorageClass
+			if valid := globalTierConfigMgr.IsTierValid(rule.NoncurrentVersionTransition.StorageClass); !valid {
+				return errInvalidStorageClass
+			}
 		}
 	}
 	return nil
 }
 
+// enqueueTransitionImmediate enqueues obj for transition if eligible.
+// This is to be called after a successful upload of an object (version).
+func enqueueTransitionImmediate(obj ObjectInfo) {
+	if lc, err := globalLifecycleSys.Get(obj.Bucket); err == nil {
+		switch lc.ComputeAction(obj.ToLifecycleOpts()) {
+		case lifecycle.TransitionAction, lifecycle.TransitionVersionAction:
+			globalTransitionState.queueTransitionTask(obj)
+		}
+	}
+}
+
 // expireAction represents different actions to be performed on expiry of a
 // restored/transitioned object
 type expireAction int
@@ -702,17 +717,16 @@ func isRestoredObjectOnDisk(meta map[string]string) (onDisk bool) {
 // ToLifecycleOpts returns lifecycle.ObjectOpts value for oi.
 func (oi ObjectInfo) ToLifecycleOpts() lifecycle.ObjectOpts {
 	return lifecycle.ObjectOpts{
-		Name:                   oi.Name,
+		Name:             oi.Name,
-		UserTags:               oi.UserTags,
+		UserTags:         oi.UserTags,
-		VersionID:              oi.VersionID,
+		VersionID:        oi.VersionID,
-		ModTime:                oi.ModTime,
+		ModTime:          oi.ModTime,
-		IsLatest:               oi.IsLatest,
+		IsLatest:         oi.IsLatest,
-		NumVersions:            oi.NumVersions,
+		NumVersions:      oi.NumVersions,
-		DeleteMarker:           oi.DeleteMarker,
+		DeleteMarker:     oi.DeleteMarker,
-		SuccessorModTime:       oi.SuccessorModTime,
+		SuccessorModTime: oi.SuccessorModTime,
-		RestoreOngoing:         oi.RestoreOngoing,
+		RestoreOngoing:   oi.RestoreOngoing,
-		RestoreExpires:         oi.RestoreExpires,
+		RestoreExpires:   oi.RestoreExpires,
-		TransitionStatus:       oi.TransitionedObject.Status,
+		TransitionStatus: oi.TransitionedObject.Status,
-		RemoteTiersImmediately: globalDebugRemoteTiersImmediately,
 	}
 }

cmd/common-main.go

@@ -598,10 +598,6 @@ func handleCommonEnvVars() {
 		}
 		GlobalKMS = KMS
 	}
-
-	if tiers := env.Get("_MINIO_DEBUG_REMOTE_TIERS_IMMEDIATELY", ""); tiers != "" {
-		globalDebugRemoteTiersImmediately = strings.Split(tiers, ",")
-	}
 }
 
 func logStartupMessage(msg string) {

cmd/data-scanner.go

@@ -879,18 +879,17 @@ func (i *scannerItem) applyLifecycle(ctx context.Context, o ObjectLayer, oi Obje
 	versionID := oi.VersionID
 	action := i.lifeCycle.ComputeAction(
 		lifecycle.ObjectOpts{
-			Name:                   i.objectPath(),
+			Name:             i.objectPath(),
-			UserTags:               oi.UserTags,
+			UserTags:         oi.UserTags,
-			ModTime:                oi.ModTime,
+			ModTime:          oi.ModTime,
-			VersionID:              oi.VersionID,
+			VersionID:        oi.VersionID,
-			DeleteMarker:           oi.DeleteMarker,
+			DeleteMarker:     oi.DeleteMarker,
-			IsLatest:               oi.IsLatest,
+			IsLatest:         oi.IsLatest,
-			NumVersions:            oi.NumVersions,
+			NumVersions:      oi.NumVersions,
-			SuccessorModTime:       oi.SuccessorModTime,
+			SuccessorModTime: oi.SuccessorModTime,
-			RestoreOngoing:         oi.RestoreOngoing,
+			RestoreOngoing:   oi.RestoreOngoing,
-			RestoreExpires:         oi.RestoreExpires,
+			RestoreExpires:   oi.RestoreExpires,
-			TransitionStatus:       oi.TransitionedObject.Status,
+			TransitionStatus: oi.TransitionedObject.Status,
-			RemoteTiersImmediately: globalDebugRemoteTiersImmediately,
 		})
 	if i.debug {
 		if versionID != "" {

cmd/globals.go

@@ -321,7 +321,6 @@ var (
 
 	globalConsoleSrv *restapi.Server
 
-	globalDebugRemoteTiersImmediately []string
 	// Add new variable global values here.
 )
 

cmd/object-handlers.go

@@ -1507,6 +1507,11 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 		UserAgent:    r.UserAgent(),
 		Host:         handlers.GetSourceIP(r),
 	})
+	if !globalTierConfigMgr.Empty() {
+		// Schedule object for immediate transition if eligible.
+		enqueueTransitionImmediate(objInfo)
+	}
+
 }
 
 // PutObjectHandler - PUT Object
@@ -1851,6 +1856,8 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 
 	// Remove the transitioned object whose object version is being overwritten.
 	if !globalTierConfigMgr.Empty() {
+		// Schedule object for immediate transition if eligible.
+		enqueueTransitionImmediate(objInfo)
 		logger.LogIf(ctx, os.Sweep())
 	}
 }
@@ -3292,6 +3299,8 @@ func (api objectAPIHandlers) CompleteMultipartUploadHandler(w http.ResponseWrite
 
 	// Remove the transitioned object whose object version is being overwritten.
 	if !globalTierConfigMgr.Empty() {
+		// Schedule object for immediate transition if eligible.
+		enqueueTransitionImmediate(objInfo)
 		logger.LogIf(ctx, os.Sweep())
 	}
 }

internal/bucket/lifecycle/lifecycle.go

@@ -137,7 +137,7 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool {
 		if rule.NoncurrentVersionExpiration.NoncurrentDays > 0 {
 			return true
 		}
-		if rule.NoncurrentVersionTransition.NoncurrentDays > 0 {
+		if !rule.NoncurrentVersionTransition.IsNull() {
 			return true
 		}
 		if rule.Expiration.IsNull() && rule.Transition.IsNull() {
@@ -146,12 +146,16 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool {
 		if !rule.Expiration.IsDateNull() && rule.Expiration.Date.Before(time.Now()) {
 			return true
 		}
+		if !rule.Expiration.IsDaysNull() {
+			return true
+		}
 		if !rule.Transition.IsDateNull() && rule.Transition.Date.Before(time.Now()) {
 			return true
 		}
-		if !rule.Expiration.IsDaysNull() || !rule.Transition.IsDaysNull() {
+		if !rule.Transition.IsNull() { // this allows for Transition.Days to be zero.
 			return true
 		}
+
 	}
 	return false
 }
@@ -175,6 +179,7 @@ func (lc Lifecycle) Validate() error {
 	if len(lc.Rules) == 0 {
 		return errLifecycleNoRule
 	}
+
 	// Validate all the rules in the lifecycle config
 	for _, r := range lc.Rules {
 		if err := r.Validate(); err != nil {
@@ -229,7 +234,7 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule {
 		// The NoncurrentVersionTransition action requests MinIO to transition
 		// noncurrent versions of objects x days after the objects become
 		// noncurrent.
-		if !rule.NoncurrentVersionTransition.IsDaysNull() {
+		if !rule.NoncurrentVersionTransition.IsNull() {
 			rules = append(rules, rule)
 			continue
 		}
@@ -247,29 +252,17 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule {
 // ObjectOpts provides information to deduce the lifecycle actions
 // which can be triggered on the resultant object.
 type ObjectOpts struct {
-	Name                   string
+	Name             string
-	UserTags               string
+	UserTags         string
-	ModTime                time.Time
+	ModTime          time.Time
-	VersionID              string
+	VersionID        string
-	IsLatest               bool
+	IsLatest         bool
-	DeleteMarker           bool
+	DeleteMarker     bool
-	NumVersions            int
+	NumVersions      int
-	SuccessorModTime       time.Time
+	SuccessorModTime time.Time
-	TransitionStatus       string
+	TransitionStatus string
-	RestoreOngoing         bool
+	RestoreOngoing   bool
-	RestoreExpires         time.Time
+	RestoreExpires   time.Time
-	RemoteTiersImmediately []string // strictly for debug only
-}
-
-// doesMatchDebugTiers returns true if tier matches one of the debugTiers, false
-// otherwise.
-func doesMatchDebugTiers(tier string, debugTiers []string) bool {
-	for _, t := range debugTiers {
-		if strings.ToUpper(tier) == strings.ToUpper(t) {
-			return true
-		}
-	}
-	return false
 }
 
 // ExpiredObjectDeleteMarker returns true if an object version referred to by o
@@ -316,7 +309,7 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 			}
 		}
 
-		if !rule.NoncurrentVersionTransition.IsDaysNull() {
+		if !rule.NoncurrentVersionTransition.IsNull() {
 			if obj.VersionID != "" && !obj.IsLatest && !obj.SuccessorModTime.IsZero() && !obj.DeleteMarker && obj.TransitionStatus != TransitionComplete {
 				// Non current versions should be transitioned if their age exceeds non current days configuration
 				// https://docs.aws.amazon.com/AmazonS3/latest/dev/intro-lifecycle-rules.html#intro-lifecycle-rules-actions
@@ -324,11 +317,6 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 					return TransitionVersionAction
 				}
 
-				// this if condition is strictly for debug purposes to force immediate
-				// transition to remote tier if _MINIO_DEBUG_REMOTE_TIERS_IMMEDIATELY is set
-				if doesMatchDebugTiers(rule.NoncurrentVersionTransition.StorageClass, obj.RemoteTiersImmediately) {
-					return TransitionVersionAction
-				}
 			}
 		}
 
@@ -346,21 +334,10 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 			}
 
 			if obj.TransitionStatus != TransitionComplete {
-				switch {
+				if due, ok := rule.Transition.NextDue(obj); ok {
-				case !rule.Transition.IsDateNull():
+					if time.Now().UTC().After(due) {
-					if time.Now().UTC().After(rule.Transition.Date.Time) {
 						action = TransitionAction
 					}
-				case !rule.Transition.IsDaysNull():
-					if time.Now().UTC().After(ExpectedExpiryTime(obj.ModTime, int(rule.Transition.Days))) {
-						action = TransitionAction
-					}
-
-				}
-				// this if condition is strictly for debug purposes to force immediate
-				// transition to remote tier if _MINIO_DEBUG_REMOTE_TIERS_IMMEDIATELY is set
-				if !rule.Transition.IsNull() && doesMatchDebugTiers(rule.Transition.StorageClass, obj.RemoteTiersImmediately) {
-					action = TransitionAction
 				}
 
 				if !obj.RestoreExpires.IsZero() && time.Now().After(obj.RestoreExpires) {

internal/bucket/lifecycle/lifecycle_test.go

@@ -104,6 +104,12 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) {
 			expectedParsingErr:    nil,
 			expectedValidationErr: nil,
 		},
+		// Lifecycle with zero Transition Days
+		{
+			inputConfig:           `<LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Rule><ID>rule</ID><Filter></Filter><Status>Enabled</Status><Transition><Days>0</Days><StorageClass>S3TIER-1</StorageClass></Transition></Rule></LifecycleConfiguration>`,
+			expectedParsingErr:    nil,
+			expectedValidationErr: nil,
+		},
 	}
 
 	for i, tc := range testCases {
@@ -119,7 +125,7 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) {
 			}
 			err = lc.Validate()
 			if err != tc.expectedValidationErr {
-				t.Fatalf("%d: Expected %v during parsing but got %v", i+1, tc.expectedValidationErr, err)
+				t.Fatalf("%d: Expected %v during validation but got %v", i+1, tc.expectedValidationErr, err)
 			}
 		})
 	}
@@ -146,7 +152,7 @@ func TestMarshalLifecycleConfig(t *testing.T) {
 				Status:                      "Enabled",
 				Filter:                      Filter{Prefix: Prefix{string: "prefix-1", set: true}},
 				Expiration:                  Expiration{Date: midnightTS},
-				NoncurrentVersionTransition: NoncurrentVersionTransition{NoncurrentDays: 2, StorageClass: "TEST"},
+				NoncurrentVersionTransition: NoncurrentVersionTransition{NoncurrentDays: TransitionDays(2), StorageClass: "TEST"},
 			},
 		},
 	}
@@ -380,6 +386,13 @@ func TestComputeActions(t *testing.T) {
 			isExpiredDelMarker: true,
 			expectedAction:     DeleteVersionAction,
 		},
+		// Should not delete expired marker if its time has not come yet
+		{
+			inputConfig:    `<BucketLifecycleConfiguration><Rule><Filter></Filter><Status>Enabled</Status><Transition><Days>0</Days><StorageClass>S3TIER-1</StorageClass></Transition></Rule></BucketLifecycleConfiguration>`,
+			objectName:     "foodir/fooobject",
+			objectModTime:  time.Now().UTC(), // Created now
+			expectedAction: TransitionAction,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -441,6 +454,16 @@ func TestHasActiveRules(t *testing.T) {
 			prefix:         "foodir/foobject",
 			expectedNonRec: false, expectedRec: false,
 		},
+		{
+			inputConfig:    `<LifecycleConfiguration><Rule><Status>Enabled</Status><Transition><StorageClass>S3TIER-1</StorageClass></Transition></Rule></LifecycleConfiguration>`,
+			prefix:         "foodir/foobject/foo.txt",
+			expectedNonRec: true, expectedRec: true,
+		},
+		{
+			inputConfig:    `<LifecycleConfiguration><Rule><Status>Enabled</Status><NoncurrentVersionTransition><StorageClass>S3TIER-1</StorageClass></NoncurrentVersionTransition></Rule></LifecycleConfiguration>`,
+			prefix:         "foodir/foobject/foo.txt",
+			expectedNonRec: true, expectedRec: true,
+		},
 	}
 
 	for i, tc := range testCases {
@@ -486,7 +509,7 @@ func TestSetPredictionHeaders(t *testing.T) {
 				ID:     "rule-3",
 				Status: "Enabled",
 				NoncurrentVersionTransition: NoncurrentVersionTransition{
-					NoncurrentDays: ExpirationDays(5),
+					NoncurrentDays: TransitionDays(5),
 					StorageClass:   "TIER-2",
 					set:            true,
 				},
@@ -559,7 +582,7 @@ func TestTransitionTier(t *testing.T) {
 				ID:     "rule-2",
 				Status: "Enabled",
 				NoncurrentVersionTransition: NoncurrentVersionTransition{
-					NoncurrentDays: ExpirationDays(3),
+					NoncurrentDays: TransitionDays(3),
 					StorageClass:   "TIER-2",
 				},
 			},

internal/bucket/lifecycle/noncurrentversion.go

@@ -70,7 +70,7 @@ func (n NoncurrentVersionExpiration) Validate() error {
 
 // NoncurrentVersionTransition - an action for lifecycle configuration rule.
 type NoncurrentVersionTransition struct {
-	NoncurrentDays ExpirationDays `xml:"NoncurrentDays"`
+	NoncurrentDays TransitionDays `xml:"NoncurrentDays"`
 	StorageClass   string         `xml:"StorageClass"`
 	set            bool
 }
@@ -78,18 +78,13 @@ type NoncurrentVersionTransition struct {
 // MarshalXML is extended to leave out
 // <NoncurrentVersionTransition></NoncurrentVersionTransition> tags
 func (n NoncurrentVersionTransition) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
-	if n.NoncurrentDays == ExpirationDays(0) {
+	if n.IsNull() {
 		return nil
 	}
 	type noncurrentVersionTransitionWrapper NoncurrentVersionTransition
 	return e.EncodeElement(noncurrentVersionTransitionWrapper(n), start)
 }
 
-// IsDaysNull returns true if days field is null
-func (n NoncurrentVersionTransition) IsDaysNull() bool {
-	return n.NoncurrentDays == ExpirationDays(0)
-}
-
 // UnmarshalXML decodes NoncurrentVersionExpiration
 func (n *NoncurrentVersionTransition) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error {
 	type noncurrentVersionTransitionWrapper NoncurrentVersionTransition
@@ -103,12 +98,18 @@ func (n *NoncurrentVersionTransition) UnmarshalXML(d *xml.Decoder, startElement
 	return nil
 }
 
+// IsNull returns true if NoncurrentTransition doesn't refer to any storage-class.
+// Note: It supports immediate transition, i.e zero noncurrent days.
+func (n NoncurrentVersionTransition) IsNull() bool {
+	return n.StorageClass == ""
+}
+
 // Validate returns an error with wrong value
 func (n NoncurrentVersionTransition) Validate() error {
 	if !n.set {
 		return nil
 	}
-	if int(n.NoncurrentDays) <= 0 || n.StorageClass == "" {
+	if n.StorageClass == "" {
 		return errXMLNotWellFormed
 	}
 	return nil
@@ -117,10 +118,12 @@ func (n NoncurrentVersionTransition) Validate() error {
 // NextDue returns upcoming NoncurrentVersionTransition date for obj if
 // applicable, returns false otherwise.
 func (n NoncurrentVersionTransition) NextDue(obj ObjectOpts) (time.Time, bool) {
-	switch {
+	if obj.IsLatest || n.StorageClass == "" {
-	case obj.IsLatest, n.IsDaysNull():
 		return time.Time{}, false
 	}
-
+	// Days == 0 indicates immediate tiering, i.e object is eligible for tiering since it became noncurrent.
+	if n.NoncurrentDays == 0 {
+		return obj.SuccessorModTime, true
+	}
 	return ExpectedExpiryTime(obj.SuccessorModTime, int(n.NoncurrentDays)), true
 }

internal/bucket/lifecycle/transition.go

@@ -25,7 +25,7 @@ import (
 var (
 	errTransitionInvalidDays     = Errorf("Days must be 0 or greater when used with Transition")
 	errTransitionInvalidDate     = Errorf("Date must be provided in ISO 8601 format")
-	errTransitionInvalid         = Errorf("Exactly one of Days (0 or greater) or Date (positive ISO 8601 format) should be present inside Expiration.")
+	errTransitionInvalid         = Errorf("Exactly one of Days (0 or greater) or Date (positive ISO 8601 format) should be present in Transition.")
 	errTransitionDateNotMidnight = Errorf("'Date' must be at midnight GMT")
 )
 
@@ -76,24 +76,23 @@ type TransitionDays int
 // UnmarshalXML parses number of days from Transition and validates if
 // >= 0
 func (tDays *TransitionDays) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error {
-	var numDays int
+	var days int
-	err := d.DecodeElement(&numDays, &startElement)
+	err := d.DecodeElement(&days, &startElement)
 	if err != nil {
 		return err
 	}
-	if numDays < 0 {
+
+	if days < 0 {
 		return errTransitionInvalidDays
 	}
-	*tDays = TransitionDays(numDays)
+	*tDays = TransitionDays(days)
+
 	return nil
 }
 
 // MarshalXML encodes number of days to expire if it is non-zero and
 // encodes empty string otherwise
 func (tDays TransitionDays) MarshalXML(e *xml.Encoder, startElement xml.StartElement) error {
-	if tDays == 0 {
-		return nil
-	}
 	return e.EncodeElement(int(tDays), startElement)
 }
 
@@ -135,25 +134,16 @@ func (t Transition) Validate() error {
 		return nil
 	}
 
-	if t.IsDaysNull() && t.IsDateNull() {
+	if !t.IsDateNull() && t.Days > 0 {
-		return errXMLNotWellFormed
-	}
-
-	// Both transition days and date are specified
-	if !t.IsDaysNull() && !t.IsDateNull() {
 		return errTransitionInvalid
 	}
+
 	if t.StorageClass == "" {
 		return errXMLNotWellFormed
 	}
 	return nil
 }
 
-// IsDaysNull returns true if days field is null
-func (t Transition) IsDaysNull() bool {
-	return t.Days == TransitionDays(0)
-}
-
 // IsDateNull returns true if date field is null
 func (t Transition) IsDateNull() bool {
 	return t.Date.Time.IsZero()
@@ -161,22 +151,23 @@ func (t Transition) IsDateNull() bool {
 
 // IsNull returns true if both date and days fields are null
 func (t Transition) IsNull() bool {
-	return t.IsDaysNull() && t.IsDateNull()
+	return t.StorageClass == ""
 }
 
 // NextDue returns upcoming transition date for obj and true if applicable,
 // returns false otherwise.
 func (t Transition) NextDue(obj ObjectOpts) (time.Time, bool) {
-	if !obj.IsLatest {
+	if !obj.IsLatest || t.IsNull() {
 		return time.Time{}, false
 	}
 
-	switch {
+	if !t.IsDateNull() {
-	case !t.IsDateNull():
 		return t.Date.Time, true
-	case !t.IsDaysNull():
-		return ExpectedExpiryTime(obj.ModTime, int(t.Days)), true
 	}
 
-	return time.Time{}, false
+	// Days == 0 indicates immediate tiering, i.e object is eligible for tiering since its creation.
+	if t.Days == 0 {
+		return obj.ModTime, true
+	}
+	return ExpectedExpiryTime(obj.ModTime, int(t.Days)), true
 }

internal/bucket/lifecycle/transition_test.go

@@ -0,0 +1,93 @@
+// Copyright (c) 2015-2021 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package lifecycle
+
+import (
+	"encoding/xml"
+	"testing"
+)
+
+func TestTransitionUnmarshalXML(t *testing.T) {
+	trTests := []struct {
+		input string
+		err   error
+	}{
+		{
+			input: `<Transition>
+			<Days>0</Days>
+			<StorageClass>S3TIER-1</StorageClass>
+		  </Transition>`,
+			err: nil,
+		},
+		{
+			input: `<Transition>
+			<Days>1</Days>
+			<Date>2021-01-01T00:00:00Z</Date>
+			<StorageClass>S3TIER-1</StorageClass>
+		  </Transition>`,
+			err: errTransitionInvalid,
+		},
+		{
+			input: `<Transition>
+			<Days>1</Days>
+		  </Transition>`,
+			err: errXMLNotWellFormed,
+		},
+	}
+
+	for i, tc := range trTests {
+		var tr Transition
+		err := xml.Unmarshal([]byte(tc.input), &tr)
+		if err != nil {
+			t.Fatalf("%d: xml unmarshal failed with %v", i+1, err)
+		}
+		if err = tr.Validate(); err != tc.err {
+			t.Fatalf("%d: Invalid transition %v: err %v", i+1, tr, err)
+		}
+	}
+
+	ntrTests := []struct {
+		input string
+		err   error
+	}{
+		{
+			input: `<NoncurrentVersionTransition>
+			<NoncurrentDays>0</NoncurrentDays>
+			<StorageClass>S3TIER-1</StorageClass>
+		  </NoncurrentVersionTransition>`,
+			err: nil,
+		},
+		{
+			input: `<NoncurrentVersionTransition>
+			<Days>1</Days>
+		  </NoncurrentVersionTransition>`,
+			err: errXMLNotWellFormed,
+		},
+	}
+
+	for i, tc := range ntrTests {
+		var ntr NoncurrentVersionTransition
+		err := xml.Unmarshal([]byte(tc.input), &ntr)
+		if err != nil {
+			t.Fatalf("%d: xml unmarshal failed with %v", i+1, err)
+		}
+		if err = ntr.Validate(); err != tc.err {
+			t.Fatalf("%d: Invalid noncurrent version transition %v: err %v", i+1, ntr, err)
+		}
+	}
+}