-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Upgrade kubernetes dependencies to 1.15.3 #1808
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,31 +18,43 @@ package output | |
|
||
import ( | ||
"fmt" | ||
"io" | ||
"regexp" | ||
"sort" | ||
"time" | ||
|
||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/runtime" | ||
"k8s.io/apimachinery/pkg/util/duration" | ||
"k8s.io/kubernetes/pkg/printers" | ||
|
||
velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" | ||
) | ||
|
||
var ( | ||
backupColumns = []string{"NAME", "STATUS", "CREATED", "EXPIRES", "STORAGE LOCATION", "SELECTOR"} | ||
backupColumns = []metav1.TableColumnDefinition{ | ||
// name needs Type and Format defined for the decorator to identify it: | ||
// https://github.com/kubernetes/kubernetes/blob/v1.15.3/pkg/printers/tableprinter.go#L204 | ||
{Name: "Name", Type: "string", Format: "name"}, | ||
{Name: "Status"}, | ||
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've decided to omit the type/format/description for all the other columns, I can't seem to find where they are used and we didn't have them before. |
||
{Name: "Created"}, | ||
{Name: "Expires"}, | ||
{Name: "Storage Location"}, | ||
{Name: "Selector"}, | ||
} | ||
) | ||
|
||
func printBackupList(list *velerov1api.BackupList, w io.Writer, options printers.PrintOptions) error { | ||
func printBackupList(list *velerov1api.BackupList, options printers.PrintOptions) ([]metav1.TableRow, error) { | ||
sortBackupsByPrefixAndTimestamp(list) | ||
rows := make([]metav1.TableRow, 0, len(list.Items)) | ||
|
||
for i := range list.Items { | ||
if err := printBackup(&list.Items[i], w, options); err != nil { | ||
return err | ||
r, err := printBackup(&list.Items[i], options) | ||
if err != nil { | ||
return nil, err | ||
} | ||
rows = append(rows, r...) | ||
} | ||
return nil | ||
return rows, nil | ||
} | ||
|
||
// sort by default alphabetically, but if backups stem from a common schedule | ||
|
@@ -71,13 +83,9 @@ func sortBackupsByPrefixAndTimestamp(list *velerov1api.BackupList) { | |
}) | ||
} | ||
|
||
func printBackup(backup *velerov1api.Backup, w io.Writer, options printers.PrintOptions) error { | ||
name := printers.FormatResourceName(options.Kind, backup.Name, options.WithKind) | ||
|
||
if options.WithNamespace { | ||
if _, err := fmt.Fprintf(w, "%s\t", backup.Namespace); err != nil { | ||
return err | ||
} | ||
func printBackup(backup *velerov1api.Backup, options printers.PrintOptions) ([]metav1.TableRow, error) { | ||
row := metav1.TableRow{ | ||
Object: runtime.RawExtension{Object: backup}, | ||
} | ||
|
||
expiration := backup.Status.Expiration.Time | ||
|
@@ -103,16 +111,9 @@ func printBackup(backup *velerov1api.Backup, w io.Writer, options printers.Print | |
|
||
location := backup.Spec.StorageLocation | ||
|
||
if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s", name, status, backup.Status.StartTimestamp.Time, humanReadableTimeFromNow(expiration), location, metav1.FormatLabelSelector(backup.Spec.LabelSelector)); err != nil { | ||
return err | ||
} | ||
|
||
if _, err := fmt.Fprint(w, printers.AppendLabels(backup.Labels, options.ColumnLabels)); err != nil { | ||
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 don't see where L110-115 are getting carried over - is this handled for us, or do we need to retain some custom code? Same question for the other printers as well. 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. this is to support the 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. Sorry, should have made that more clear. It's now handled for us in https://github.com/kubernetes/kubernetes/blob/v1.15.3/pkg/printers/tableprinter.go#L190, see the logic around I've also manually tested each command and variations of show-labels/label-columns to ensure it matches the output from before. 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. OK, awesome! I figured you were on top of this, just didn't see it myself :) 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. Yeah it took me a while to figure out too, it was pretty hidden there. Overall I think the improvements to the printer package are really great, it's so much simpler to use now :) |
||
return err | ||
} | ||
row.Cells = append(row.Cells, backup.Name, status, backup.Status.StartTimestamp.Time, humanReadableTimeFromNow(expiration), location, metav1.FormatLabelSelector(backup.Spec.LabelSelector)) | ||
|
||
_, err := fmt.Fprint(w, printers.AppendAllLabels(options.ShowLabels, backup.Labels)) | ||
return err | ||
return []metav1.TableRow{row}, nil | ||
} | ||
|
||
func humanReadableTimeFromNow(when time.Time) string { | ||
|
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.
FYI I'm using capitalisation for the Name field as this is what upstream is doing for core resources: https://github.com/kubernetes/kubernetes/blob/d2e2337744072553ae7756ba972607ffb6532a85/pkg/printers/internalversion/printers.go#L80