Upgrade kubernetes dependencies to 1.15.3 #1808

prydonius · 2019-08-26T23:55:20Z

Split into two commits to make this easier to review. The first updates the dependency versions in Gopkg and the Dockerfile. The second commit updates Velero's printer logic to work with the upstream changes to the printer package.

Signed-off-by: Adnan Abdulhussein <[email protected]>

prydonius · 2019-08-27T00:00:25Z

pkg/cmd/util/output/backup_printer.go

+		// 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"},


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.

prydonius · 2019-08-27T00:01:48Z

pkg/cmd/util/output/backup_printer.go

+	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"},


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

Signed-off-by: Adnan Abdulhussein <[email protected]>

prydonius · 2019-08-27T00:11:28Z

Will look into why the code generator in the build container is failing tomorrow, let me know if you have any pointers for that (looks like we might just need to pull some new dependencies, but not sure why they wouldn't just be pulled from vendor?)

carlisia · 2019-08-27T00:48:15Z

I googled a bit and found this: kubernetes-sigs/kubebuilder#359 - however, I see that we already export GOPATH. And it seems the path is the same as other builds. I know this is zero help, just thinking out loud.

skriss · 2019-08-27T15:01:14Z

Will look into why the code generator in the build container is failing tomorrow, let me know if you have any pointers for that (looks like we might just need to pull some new dependencies, but not sure why they wouldn't just be pulled from vendor?)

Looks like the vendor/ directory in this repo was removed in 1.15 as part of switching from Godeps to modules, and modules are disabled by default when within $GOPATH, so we'll either want to move stuff out of the GOPATH to get automatic module support, or explicitly turn it on. Not sure which is easier.

carlisia · 2019-08-27T15:02:51Z

We're gonna have to turn on go mod sooner or later? 🤷‍♂

nrb · 2019-08-27T16:06:40Z

We're gonna have to turn on go mod sooner or later?

Yeah, we will. I'd personally prefer to wait if we can, as go modules still don't seem fully ready. Especially if you look at surrounding tooling around them, like gopls.

skriss · 2019-08-27T16:10:18Z

Yeah, we don't need to migrate to modules for velero just for this. By "turn it on" I meant turning on module support within GOPATH in the builder container for the purposes of installing/running the code generator.

prydonius · 2019-08-27T18:23:52Z

Adding GO111MODULE=on to hack/update-generated-crd-code.sh results in the following error when running make update:

go: extracting golang.org/x/net v0.0.0-20190812203447-cdfb69ac37fc
go: extracting github.com/gogo/protobuf v1.2.2-0.20190723190241-65acae22fc9d
go: downloading golang.org/x/text v0.3.2
go: extracting golang.org/x/text v0.3.2
go: writing go.sum: open /go/src/k8s.io/code-generator/go.sum261091864.tmp: permission denied

make[1]: *** [shell] Error 255
make: *** [update] Error 2

Looks like we're running the container as a different user when running make update: https://github.com/heptio/velero/blob/ad2e162283e9643b8ea26f360c3a2303d3e66a05/Makefile#L120, and this is why we're getting that permission error.

I'm also trying to see if we can download the modules in the Dockerfile when building the image, as this will then get cached and we can avoid having to download all the modules everytime we run the code-generator.

Signed-off-by: Adnan Abdulhussein <[email protected]>

prydonius · 2019-08-27T19:01:41Z

Okay seems like the suggested way to do this is to use go mod vendor to vendor the modules so it acts like before 1.15 (https://github.com/kubernetes/sample-apiserver#when-using-go-111-modules). I'm able to do this in the Dockerfile so that we can cache it and don't have to do it everytime we run make update.

prydonius · 2019-08-27T19:12:06Z

CI is now green, PTAL :)!

skriss

Mostly looks good - just one question on the printers

skriss · 2019-08-27T19:20:26Z

pkg/cmd/util/output/backup_printer.go

-		return err
-	}
-
-	if _, err := fmt.Fprint(w, printers.AppendLabels(backup.Labels, options.ColumnLabels)); err != nil {


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.

this is to support the --show-labels and --label-columns flags.

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 options.ShowLabels and options.ColumnLabels which we set when we initialise the printer: https://github.com/heptio/velero/blob/772459a8b40fd3d26e98397e27302d235b4d1584/pkg/cmd/util/output/output.go#L174-L177.

I've also manually tested each command and variations of show-labels/label-columns to ensure it matches the output from before.

OK, awesome! I figured you were on top of this, just didn't see it myself :)

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 :)

prydonius · 2019-08-27T19:41:52Z

pkg/cmd/util/output/output.go

@@ -172,15 +172,11 @@ func printTable(cmd *cobra.Command, obj runtime.Object) (bool, error) {
 // Velero objects.
 func NewPrinter(cmd *cobra.Command) (*printers.HumanReadablePrinter, error) {
 	options := printers.PrintOptions{
-		NoHeaders:    flag.GetOptionalBoolFlag(cmd, "no-headers"),


@skriss FYI I removed this because I couldn't see that flag defined anywhere, let me know if I'm missing something

seems fine, I'm not aware of any support for this

skriss

LGTM

carlisia

👍

* upstream/master: (118 commits) restore: rename PV when remapping a namespace if PV exists in-cluster (vmware-tanzu#1779) when backing up PVCs with restic, explicitly specify --parent (vmware-tanzu#1807) Unit tests for restic restore (vmware-tanzu#1747) Upgrade kubernetes dependencies to 1.15.3 (vmware-tanzu#1808) create backups from schedules using velero create backup (vmware-tanzu#1734) remove calls to restic check before/after prune (vmware-tanzu#1794) Propose adding feature flags to velero restic backup and restore progress proposal (vmware-tanzu#1765) allow custom restic repo prefix to be specified in BSL config error if restic repo identifier can't be determined update nokogiri dep for website update links on website home page for latest release (vmware-tanzu#1789) Velero 1.1 blog post v1.1.0 changelog fix error formatting Revert "allow self signed certs with insecureSkipVerify (vmware-tanzu#1769)" allow self signed certs with insecureSkipVerify (vmware-tanzu#1769) v1.1.0 docs Add the prefix to BSL config map so that object stores can use it when initializing (vmware-tanzu#1767) add stable/velero to helm commands ...

* jesse/20190828_merge: (511 commits) fix(ci): Update arm32 target. feat(ci): Auto-build restic-restore-helper image in CI. restore: rename PV when remapping a namespace if PV exists in-cluster (vmware-tanzu#1779) when backing up PVCs with restic, explicitly specify --parent (vmware-tanzu#1807) Unit tests for restic restore (vmware-tanzu#1747) Upgrade kubernetes dependencies to 1.15.3 (vmware-tanzu#1808) create backups from schedules using velero create backup (vmware-tanzu#1734) remove calls to restic check before/after prune (vmware-tanzu#1794) Propose adding feature flags to velero restic backup and restore progress proposal (vmware-tanzu#1765) allow custom restic repo prefix to be specified in BSL config error if restic repo identifier can't be determined update nokogiri dep for website update links on website home page for latest release (vmware-tanzu#1789) Velero 1.1 blog post v1.1.0 changelog fix error formatting Revert "allow self signed certs with insecureSkipVerify (vmware-tanzu#1769)" allow self signed certs with insecureSkipVerify (vmware-tanzu#1769) v1.1.0 docs ...

bump kubernetes dependency to 1.15.3

14a1eb2

Signed-off-by: Adnan Abdulhussein <[email protected]>

prydonius requested review from carlisia, nrb and skriss August 26, 2019 23:55

prydonius commented Aug 27, 2019

View reviewed changes

migrate to new k8s printers functions

ad2e162

Signed-off-by: Adnan Abdulhussein <[email protected]>

prydonius force-pushed the 1795-k8s-1.15-deps branch from 2039a27 to ad2e162 Compare August 27, 2019 00:03

Adnan Abdulhussein added 2 commits August 27, 2019 11:59

vendor go modules for code-generator

44bc107

Signed-off-by: Adnan Abdulhussein <[email protected]>

generated: code-generator updates

772459a

Signed-off-by: Adnan Abdulhussein <[email protected]>

skriss reviewed Aug 27, 2019

View reviewed changes

prydonius commented Aug 27, 2019

View reviewed changes

skriss approved these changes Aug 27, 2019

View reviewed changes

carlisia approved these changes Aug 27, 2019

View reviewed changes

carlisia merged commit 7ea065a into vmware-tanzu:master Aug 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade kubernetes dependencies to 1.15.3 #1808

Upgrade kubernetes dependencies to 1.15.3 #1808

prydonius commented Aug 26, 2019

prydonius Aug 27, 2019

prydonius Aug 27, 2019

prydonius commented Aug 27, 2019

carlisia commented Aug 27, 2019 •

edited

Loading

skriss commented Aug 27, 2019

carlisia commented Aug 27, 2019

nrb commented Aug 27, 2019

skriss commented Aug 27, 2019

prydonius commented Aug 27, 2019

prydonius commented Aug 27, 2019

prydonius commented Aug 27, 2019

skriss left a comment

skriss Aug 27, 2019

skriss Aug 27, 2019

prydonius Aug 27, 2019

skriss Aug 27, 2019

prydonius Aug 27, 2019

prydonius Aug 27, 2019

skriss Aug 27, 2019

skriss left a comment

carlisia left a comment

Upgrade kubernetes dependencies to 1.15.3 #1808

Upgrade kubernetes dependencies to 1.15.3 #1808

Conversation

prydonius commented Aug 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prydonius commented Aug 27, 2019

carlisia commented Aug 27, 2019 • edited Loading

skriss commented Aug 27, 2019

carlisia commented Aug 27, 2019

nrb commented Aug 27, 2019

skriss commented Aug 27, 2019

prydonius commented Aug 27, 2019

prydonius commented Aug 27, 2019

prydonius commented Aug 27, 2019

skriss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skriss left a comment

Choose a reason for hiding this comment

carlisia left a comment

Choose a reason for hiding this comment

carlisia commented Aug 27, 2019 •

edited

Loading