-
Notifications
You must be signed in to change notification settings - Fork 169
Add recreate-vms-created-before option to recreate and deploy commands #710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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)"` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What time zone are we enforcing here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
|
||
| 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"` | ||
|
|
@@ -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"` | ||
|
|
||
| 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
|
||||||
| 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) | ||||||
|
||||||
| return a.Format(time.RFC3339) | |
| return a.UTC().Format(time.RFC3339) |
| 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")) | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
|
||
| return recreateOpts, nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DeployCmdalways passesRecreateVMsCreatedBeforethrough to the director when the flag is set, but there’s no validation that--recreatewas also provided (even though the flag description says it’s required). Add a check to error (or automatically implyRecreate=true) when a timestamp is provided withoutopts.Recreate, otherwise the director may reject/ignore the parameter.