Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,16 @@ func (c DeployCmd) Run(opts DeployOpts) error {
}

updateOpts := boshdir.UpdateOpts{
RecreatePersistentDisks: opts.RecreatePersistentDisks,
Recreate: opts.Recreate,
Fix: opts.Fix,
SkipDrain: opts.SkipDrain,
DryRun: opts.DryRun,
Canaries: opts.Canaries,
MaxInFlight: opts.MaxInFlight,
Diff: deploymentDiff,
ForceLatestVariables: opts.ForceLatestVariables,
RecreatePersistentDisks: opts.RecreatePersistentDisks,
Recreate: opts.Recreate,
RecreateVMsCreatedBefore: opts.RecreateVMsCreatedBefore.Time,
Fix: opts.Fix,
Comment on lines 104 to +108
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeployCmd always passes RecreateVMsCreatedBefore through to the director when the flag is set, but there’s no validation that --recreate was also provided (even though the flag description says it’s required). Add a check to error (or automatically imply Recreate=true) when a timestamp is provided without opts.Recreate, otherwise the director may reject/ignore the parameter.

Copilot uses AI. Check for mistakes.
SkipDrain: opts.SkipDrain,
DryRun: opts.DryRun,
Canaries: opts.Canaries,
MaxInFlight: opts.MaxInFlight,
Diff: deploymentDiff,
ForceLatestVariables: opts.ForceLatestVariables,
}

return c.deployment.Update(bytes, updateOpts)
Expand Down
18 changes: 18 additions & 0 deletions cmd/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd_test

import (
"errors"
"time"

"github.com/cppforlife/go-patch/patch"
. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -87,6 +88,23 @@ var _ = Describe("DeployCmd", func() {
}))
})

It("deploys manifest allowing to recreate VMs created before a timestamp", func() {
deployOpts.Recreate = true
deployOpts.RecreateVMsCreatedBefore = opts.TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}

err := act()
Expect(err).ToNot(HaveOccurred())

Expect(deployment.UpdateCallCount()).To(Equal(1))

bytes, updateOpts := deployment.UpdateArgsForCall(0)
Expect(bytes).To(Equal([]byte("name: dep\n")))
Expect(updateOpts).To(Equal(boshdir.UpdateOpts{
Recreate: true,
RecreateVMsCreatedBefore: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC),
}))
})

It("deploys manifest allowing to dry_run", func() {
deployOpts.DryRun = true

Expand Down
18 changes: 10 additions & 8 deletions cmd/opts/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,12 +503,13 @@ type DeployOpts struct {

NoRedact bool `long:"no-redact" description:"Show non-redacted manifest diff"`

Recreate bool `long:"recreate" description:"Recreate all VMs in deployment"`
RecreatePersistentDisks bool `long:"recreate-persistent-disks" description:"Recreate all persistent disks in deployment"`
Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
FixReleases bool `long:"fix-releases" description:"Reupload releases in manifest and replace corrupt or missing jobs/packages"`
SkipDrain []boshdir.SkipDrain `long:"skip-drain" value-name:"[INSTANCE-GROUP[/INSTANCE-ID]]" description:"Skip running drain and pre-stop scripts for specific instance groups" optional:"true" optional-value:"*"`
SkipUploadReleases bool `long:"skip-upload-releases" description:"Skips the upload procedure for releases"`
Recreate bool `long:"recreate" description:"Recreate all VMs in deployment"`
RecreatePersistentDisks bool `long:"recreate-persistent-disks" description:"Recreate all persistent disks in deployment"`
RecreateVMsCreatedBefore TimeArg `long:"recreate-vms-created-before" description:"Only recreate VMs created before the given RFC 3339 timestamp (requires --recreate)"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What time zone are we enforcing here?

Copy link
Author

@Alphasite Alphasite Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe RFC3339 requires a timezone be specified in the timezone string: https://datatracker.ietf.org/doc/html/rfc3339#section-5.6 and UTC is the preferred format. I dont feel strongly, but i was inclined to just allow what ever timezones the spec allows (Z +/- an offset from UTC)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ill add tests to make sure it behaves the way we expect; based on yours an arams feedback.

  • make sure it handles dates without a +00:00
  • make sure all timestamps are UTC timezone internally

Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
FixReleases bool `long:"fix-releases" description:"Reupload releases in manifest and replace corrupt or missing jobs/packages"`
SkipDrain []boshdir.SkipDrain `long:"skip-drain" value-name:"[INSTANCE-GROUP[/INSTANCE-ID]]" description:"Skip running drain and pre-stop scripts for specific instance groups" optional:"true" optional-value:"*"`
SkipUploadReleases bool `long:"skip-upload-releases" description:"Skips the upload procedure for releases"`

Canaries string `long:"canaries" description:"Override manifest values for canaries"`
MaxInFlight string `long:"max-in-flight" description:"Override manifest values for max_in_flight"`
Expand Down Expand Up @@ -941,8 +942,9 @@ type RestartOpts struct {
type RecreateOpts struct {
Args AllOrInstanceGroupOrInstanceSlugArgs `positional-args:"true"`

SkipDrain bool `long:"skip-drain" description:"Skip running drain and pre-stop scripts"`
Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
SkipDrain bool `long:"skip-drain" description:"Skip running drain and pre-stop scripts"`
Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
VMsCreatedBefore TimeArg `long:"vms-created-before" description:"Only recreate VMs created before the given RFC 3339 timestamp"`

Canaries string `long:"canaries" description:"Override manifest values for canaries"`
MaxInFlight string `long:"max-in-flight" description:"Override manifest values for max_in_flight"`
Expand Down
41 changes: 41 additions & 0 deletions cmd/opts/time_arg.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package opts

import (
"time"

bosherr "github.com/cloudfoundry/bosh-utils/errors"
)

type TimeArg struct {
time.Time
}

func (a *TimeArg) UnmarshalFlag(data string) error {
// Try RFC3339 first (with timezone)
t, err := time.Parse(time.RFC3339, data)
if err != nil {
// Try RFC3339 without timezone suffix, assume UTC
// Format: "2006-01-02T15:04:05"
t, err = time.Parse("2006-01-02T15:04:05", data)
Comment on lines +15 to +19
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnmarshalFlag uses time.Parse(time.RFC3339, ...), which rejects valid RFC 3339 timestamps that include fractional seconds (e.g. 2026-01-01T00:00:00.123Z). If the flag is documented as “RFC 3339”, consider trying time.RFC3339Nano (or accepting both) so users can paste timestamps produced by common tooling.

Copilot uses AI. Check for mistakes.
if err != nil {
return bosherr.Errorf("Invalid timestamp '%s': expected RFC 3339 format (e.g., 2006-01-02T15:04:05Z or 2006-01-02T15:04:05)", data)
}
// Treat as UTC since no timezone was specified
t = t.UTC()
}
// Always store as UTC internally
a.Time = t.UTC()
return nil
}

func (a TimeArg) IsSet() bool {
return !a.IsZero()
}

func (a TimeArg) AsString() string {
if a.IsSet() {
// Always output in UTC with Z suffix for consistency
return a.Format(time.RFC3339)
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AsString’s comment says it always outputs UTC with a Z suffix, but it calls a.Format(time.RFC3339) directly. If TimeArg is constructed programmatically with a non-UTC location, this will output an offset instead of Z. Consider formatting a.UTC() (or enforcing UTC on assignment) to match the documented behavior.

Suggested change
return a.Format(time.RFC3339)
return a.UTC().Format(time.RFC3339)

Copilot uses AI. Check for mistakes.
}
return ""
}
102 changes: 102 additions & 0 deletions cmd/opts/time_arg_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package opts_test

import (
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

. "github.com/cloudfoundry/bosh-cli/v7/cmd/opts"
)

var _ = Describe("TimeArg", func() {
Describe("UnmarshalFlag", func() {
It("parses valid RFC 3339 timestamps with Z suffix", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-01-01T00:00:00Z")
Expect(err).ToNot(HaveOccurred())
Expect(arg.Time).To(Equal(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)))
})

It("parses RFC 3339 timestamps with timezone offset and converts to UTC", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-06-15T14:30:00-07:00")
Expect(err).ToNot(HaveOccurred())
Expect(arg.Time).To(Equal(time.Date(2026, 6, 15, 21, 30, 0, 0, time.UTC)))
})

It("parses RFC 3339 timestamps with +00:00 offset and converts to UTC", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-01-15T10:30:00+00:00")
Expect(err).ToNot(HaveOccurred())
Expect(arg.Time).To(Equal(time.Date(2026, 1, 15, 10, 30, 0, 0, time.UTC)))
})

It("parses timestamps without timezone suffix and treats as UTC", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-01-01T00:00:00")
Expect(err).ToNot(HaveOccurred())
Expect(arg.Time).To(Equal(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)))
})

It("parses timestamps without timezone with specific time and treats as UTC", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-06-15T14:30:45")
Expect(err).ToNot(HaveOccurred())
Expect(arg.Time).To(Equal(time.Date(2026, 6, 15, 14, 30, 45, 0, time.UTC)))
})

It("returns error for invalid timestamps", func() {
var arg TimeArg
err := arg.UnmarshalFlag("not-a-timestamp")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Invalid timestamp"))
})

It("returns error for date-only formats (no time component)", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-01-01")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Invalid timestamp"))
})
})

Describe("IsSet", func() {
It("returns false for zero time", func() {
var arg TimeArg
Expect(arg.IsSet()).To(BeFalse())
})

It("returns true for non-zero time", func() {
arg := TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
Expect(arg.IsSet()).To(BeTrue())
})
})

Describe("AsString", func() {
It("returns empty string for zero time", func() {
var arg TimeArg
Expect(arg.AsString()).To(Equal(""))
})

It("returns RFC 3339 formatted string in UTC for non-zero time", func() {
arg := TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
Expect(arg.AsString()).To(Equal("2026-01-01T00:00:00Z"))
})

It("returns UTC formatted string even when time was parsed with offset", func() {
var arg TimeArg
// Parse with -07:00 offset (14:30 PST = 21:30 UTC)
err := arg.UnmarshalFlag("2026-06-15T14:30:00-07:00")
Expect(err).ToNot(HaveOccurred())
Expect(arg.AsString()).To(Equal("2026-06-15T21:30:00Z"))
})

It("returns UTC formatted string for timestamp parsed without timezone", func() {
var arg TimeArg
err := arg.UnmarshalFlag("2026-06-15T14:30:00")
Expect(err).ToNot(HaveOccurred())
Expect(arg.AsString()).To(Equal("2026-06-15T14:30:00Z"))
})
})
})
13 changes: 7 additions & 6 deletions cmd/recreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ func (c RecreateCmd) Run(opts RecreateOpts) error {
func newRecreateOpts(opts RecreateOpts) (boshdir.RecreateOpts, error) {
if !opts.NoConverge { // converge is default, no-converge is opt-in
recreateOpts := boshdir.RecreateOpts{
SkipDrain: opts.SkipDrain,
Fix: opts.Fix,
DryRun: opts.DryRun,
Canaries: opts.Canaries,
MaxInFlight: opts.MaxInFlight,
Converge: true,
SkipDrain: opts.SkipDrain,
Fix: opts.Fix,
DryRun: opts.DryRun,
Canaries: opts.Canaries,
MaxInFlight: opts.MaxInFlight,
Converge: true,
VMsCreatedBefore: opts.VMsCreatedBefore.Time,
}
Comment on lines +39 to 43
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--vms-created-before is only applied in the converge branch. If a user runs recreate with --no-converge and provides this flag, it will be silently ignored. Consider validating and returning an error when VMsCreatedBefore is set together with --no-converge (or otherwise document/handle the behavior explicitly).

Copilot uses AI. Check for mistakes.
return recreateOpts, nil
}
Expand Down
13 changes: 13 additions & 0 deletions cmd/recreate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd_test

import (
"errors"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -115,6 +116,18 @@ var _ = Describe("RecreateCmd", func() {
Expect(recreateOpts.Fix).To(BeTrue())
})

It("can set vms_created_before", func() {
recreateOpts.VMsCreatedBefore = opts.TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}

err := act()
Expect(err).ToNot(HaveOccurred())

Expect(deployment.RecreateCallCount()).To(Equal(1))

_, recreateOpts := deployment.RecreateArgsForCall(0)
Expect(recreateOpts.VMsCreatedBefore).To(Equal(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)))
})

It("does not recreate if confirmation is rejected", func() {
ui.AskedConfirmationErr = errors.New("stop")

Expand Down
23 changes: 16 additions & 7 deletions director/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"net/url"
"strings"
"time"

bosherr "github.com/cloudfoundry/bosh-utils/errors"
)
Expand Down Expand Up @@ -138,7 +139,7 @@ func (d DeploymentImpl) Start(slug AllOrInstanceGroupOrInstanceSlug, opts StartO
if !opts.Converge {
return d.nonConvergingJobAction("start", slug, false, false, false)
}
return d.changeJobState("started", slug, false, false, false, false, opts.Canaries, opts.MaxInFlight)
return d.changeJobState("started", slug, false, false, false, false, opts.Canaries, opts.MaxInFlight, time.Time{})
}

func (d DeploymentImpl) Stop(slug AllOrInstanceGroupOrInstanceSlug, opts StopOpts) error {
Expand All @@ -150,32 +151,32 @@ func (d DeploymentImpl) Stop(slug AllOrInstanceGroupOrInstanceSlug, opts StopOpt
if opts.Hard {
state = "detached"
}
return d.changeJobState(state, slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight)
return d.changeJobState(state, slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight, time.Time{})
}

func (d DeploymentImpl) Restart(slug AllOrInstanceGroupOrInstanceSlug, opts RestartOpts) error {
if !opts.Converge {
return d.nonConvergingJobAction("restart", slug, opts.SkipDrain, false, false)
}

return d.changeJobState("restart", slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight)
return d.changeJobState("restart", slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight, time.Time{})
}

func (d DeploymentImpl) Recreate(slug AllOrInstanceGroupOrInstanceSlug, opts RecreateOpts) error {
if !opts.Converge {
return d.nonConvergingJobAction("recreate", slug, opts.SkipDrain, false, opts.Fix)
}

return d.changeJobState("recreate", slug, opts.SkipDrain, opts.Force, opts.Fix, opts.DryRun, opts.Canaries, opts.MaxInFlight)
return d.changeJobState("recreate", slug, opts.SkipDrain, opts.Force, opts.Fix, opts.DryRun, opts.Canaries, opts.MaxInFlight, opts.VMsCreatedBefore)
}

func (d DeploymentImpl) nonConvergingJobAction(action string, slug AllOrInstanceGroupOrInstanceSlug, skipDrain bool, hard bool, ignoreUnresponsiveAgent bool) error {
return d.client.NonConvergingJobAction(action, d.name, slug.Name(), slug.IndexOrID(), skipDrain, hard, ignoreUnresponsiveAgent)
}

func (d DeploymentImpl) changeJobState(state string, slug AllOrInstanceGroupOrInstanceSlug, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string) error {
func (d DeploymentImpl) changeJobState(state string, slug AllOrInstanceGroupOrInstanceSlug, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string, vmsCreatedBefore time.Time) error {
return d.client.ChangeJobState(
state, d.name, slug.Name(), slug.IndexOrID(), skipDrain, force, fix, dryRun, canaries, maxInFlight)
state, d.name, slug.Name(), slug.IndexOrID(), skipDrain, force, fix, dryRun, canaries, maxInFlight, vmsCreatedBefore)
}

func (d DeploymentImpl) ExportRelease(release ReleaseSlug, os OSVersionSlug, jobs []string) (ExportReleaseResult, error) {
Expand Down Expand Up @@ -387,7 +388,7 @@ func (c Client) NonConvergingJobAction(action string, deployment string, instanc
return nil
}

func (c Client) ChangeJobState(state, deploymentName, job, indexOrID string, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string) error {
func (c Client) ChangeJobState(state, deploymentName, job, indexOrID string, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string, vmsCreatedBefore time.Time) error {
if len(state) == 0 {
return bosherr.Error("Expected non-empty job state")
}
Expand Down Expand Up @@ -426,6 +427,10 @@ func (c Client) ChangeJobState(state, deploymentName, job, indexOrID string, ski
query.Add("max_in_flight", maxInFlight)
}

if !vmsCreatedBefore.IsZero() {
query.Add("recreate_vm_created_before", vmsCreatedBefore.Format(time.RFC3339))
}

path := fmt.Sprintf("/deployments/%s/jobs", deploymentName)

if len(job) > 0 {
Expand Down Expand Up @@ -525,6 +530,10 @@ func (c Client) UpdateDeployment(manifest []byte, opts UpdateOpts) error {
query.Add("recreate_persistent_disks", "true")
}

if !opts.RecreateVMsCreatedBefore.IsZero() {
query.Add("recreate_vm_created_before", opts.RecreateVMsCreatedBefore.Format(time.RFC3339))
}

if opts.Fix {
query.Add("fix", "true")
}
Expand Down
Loading
Loading