Update to kubernetes 1.12.4 #1882

gonzolino · 2019-01-02T12:42:47Z

What this PR does / why we need it:
This PR updates dependencies from k8s 1.11.2 to 1.12.4. Additionally code and resources were regenerated with make generate.

Special notes for your reviewer:
Please have a look at the changes in pkg/virt-controller. Changes in the new client-go version made it necessary to replace the 'stop' channel with a context.

Release note:

NONE

ovirt-infra · 2019-01-02T12:42:49Z

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

Type ci test please on this PR to trigger automated tests for it.
Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically.
If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

cynepco3hahue · 2019-01-02T13:54:03Z

@gonzolino Hi can you please check why Travis unhappy?
Probably you need to run make and commit changes to generated files.

 M pkg/api/v1/deepcopy_generated.go
 M pkg/api/v1/openapi_generated.go
 M pkg/api/v1/zz_generated.defaults.go
 M pkg/virt-launcher/virtwrap/api/deepcopy_generated.go
 M pkg/virt-launcher/virtwrap/api/zz_generated.defaults.go
The command "if [[ -n "$(git status --porcelain)" ]] ; then echo "It seems like you need to run `make`. Please run it and commit the changes"; git status --porcelain; false; fi" exited with 1.

gonzolino · 2019-01-02T14:15:39Z

@cynepco3hahue thanks for the hint! I should have re-run make generate after doing a rebase this morning 😛
Lets see if the next run works...

cynepco3hahue · 2019-01-02T14:17:09Z

ci test please

fabiand · 2019-01-04T15:49:30Z

ci test please

cynepco3hahue · 2019-01-06T12:08:02Z

@gonzolino can you please rebase

rmohr

Looks great. @mfranczy @slintes is there still something blocking us from dropping go 1.9 support? This will make go 1.10 a requirement.

mfranczy · 2019-01-07T09:05:12Z

@rmohr, no, the ds builds use the latest available golang version which is 1.10.3.

mfranczy · 2019-01-07T09:19:49Z

@rmohr although from the k8s changelog I see they upgraded to version 1.10.4,

https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.12.md#external-dependencies

kubernetes/kubernetes#68802

rmohr · 2019-01-07T09:22:40Z

@mfranczy could you give it a try and report back?

mfranczy · 2019-01-07T09:24:04Z

@rmohr sure.

cynepco3hahue · 2019-01-07T15:49:29Z

@gonzolino why does it downgrade ginkgo O_O? Do you run make commands to update dependencies?
make deps-update

gonzolino · 2019-01-07T16:16:41Z

@cynepco3hahue Turns out I'm just not smart enough to use glide correctly 😛 I was indeed using glide update directly. Using make deps-update worked like a charm. I guess the problem was the missing --strip-vendor.

I will update the PR in a moment.

cynepco3hahue · 2019-01-07T16:43:07Z

ci test please

cynepco3hahue · 2019-01-08T15:48:23Z

ci test please

cynepco3hahue · 2019-01-09T08:14:48Z

ci test please

cynepco3hahue · 2019-01-09T08:17:28Z

pkg/virt-launcher/virtwrap/api/converter.go

@@ -116,7 +116,8 @@ func Convert_v1_Disk_To_api_Disk(diskDevice *v1.Disk, disk *Disk, devicePerBus m
 func checkDirectIOFlag(path string) bool {
 	// check if fs where disk.img file is located or block device
 	// support direct i/o
-	f, err := os.OpenFile(path, syscall.O_RDONLY|syscall.O_DIRECT, 0)
+	f, err := os.OpenFile(path, syscall.O_RDONLY, 0)
+	//f, err := os.OpenFile(path, syscall.O_RDONLY|syscall.O_DIRECT, 0)


Why do you change it?
I also think some CI failures can be related to this change

Configurations with driver cache settings [It] should set appropriate cache modes

@cynepco3hahue good catch, the O_DIRECT flag must be here.

Oh thank you! What a stupid mistake on my side.
It seems like there is no syscall.O_DIRECT on macOS. I therefore removed the line to prevent an error on my laptop and then I forgot to put it back.

So at least this needs to be fixed.

mfranczy · 2019-01-09T09:01:21Z

@rmohr I was able to compile KubeVirt with golang 1.10.3 (locally and ds scratch build), functional tests passed as well (locally). From the compiler version perspective we are not blocked. When it's possible I will switch to golang 1.11.2 (I will create a PR for upstream as well).

rmohr

The O_DIRECT issue needs to be fixed.

mfranczy · 2019-01-09T19:27:23Z

ci test please

slintes · 2019-01-14T11:34:26Z

ci test please

slintes

One small change please. Other than that LGTM 👍

slintes · 2019-01-14T16:34:39Z

cluster/examples/vm-alpine-datavolume.yaml

@@ -14,6 +14,7 @@ spec:
      pvc:
        accessModes:
        - ReadWriteOnce
+        dataSource: null


I understand this new field is not required, right? If so, I would like to get rid of these null fields in the example manifests. Can you please remove it? See https://github.com/kubevirt/kubevirt/blob/master/tools/util/marshaller.go#L46

The three null fields should now be gone. I hope to code to remove them is ok!

Remove required attribute from openAPI and remove empty dataSource fields in marshaller See kubevirt#1882 (comment) for more details.

gonzolino · 2019-01-15T08:02:13Z

@rmohr @slintes Could you have another look?

slintes

lgtm, thanks! Will trigger a final CI run

slintes · 2019-01-15T09:59:06Z

tools/util/marshaller.go

+		for _, tmpl := range templates {
+			template := tmpl.(map[string]interface{})
+			_, exists, err = unstructured.NestedString(template, "spec", "pvc", "dataSource")
+			if !exists {


nice that you check if it's empty, makes it future proof 👍

slintes · 2019-01-15T10:07:18Z

ci test please

slintes

test failures look unrelated
lgtm now, but I'd like to have a final review of @davidvossel

davidvossel

Looks good overall. I just noticed a few minor things.

pkg/virt-controller/watch/application.go

tools/util/marshaller.go

Remove required attribute from openAPI and remove empty dataSource fields in marshaller See kubevirt#1882 (comment) for more details.

davidvossel

👍

davidvossel · 2019-01-17T16:46:24Z

ci test please

davidvossel · 2019-01-18T22:55:31Z

This patch is good to go. #1952 Is about to collide with these vendoring changes though. That CDI change is unfortunately pretty high priority, so we should let that in before merging this PR.

I'm suspecting there will be merge conflicts then. I'll try and help out next week to resolve the conflicts and merge this pr.

Remove required attribute from openAPI and remove empty dataSource fields in marshaller See kubevirt#1882 (comment) for more details.

Signed-off-by: David Vossel <davidvossel@gmail.com>

davidvossel · 2019-01-21T17:38:43Z

i fixed the merge conflicts. we're just waiting for CI to pass again.

davidvossel · 2019-01-21T18:41:11Z

ci test please

davidvossel · 2019-01-21T22:30:55Z

ct test please

cynepco3hahue · 2019-01-22T08:18:56Z

ci test please

cynepco3hahue · 2019-01-22T09:49:31Z

ci test please

cynepco3hahue · 2019-01-22T14:18:25Z

ci test please

davidvossel · 2019-01-22T19:47:31Z

ci test please

cynepco3hahue · 2019-01-23T07:36:03Z

I think this PR is ready to go 👍

kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL labels Jan 2, 2019

gonzolino force-pushed the kubernetes-1.12 branch from 59f104a to e836e3f Compare January 2, 2019 14:13

kubevirt-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jan 2, 2019

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 6, 2019

rmohr reviewed Jan 7, 2019

View reviewed changes

gonzolino force-pushed the kubernetes-1.12 branch from e836e3f to 1b8dc28 Compare January 7, 2019 15:43

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2019

gonzolino force-pushed the kubernetes-1.12 branch from 1b8dc28 to 62895e4 Compare January 7, 2019 16:17

gonzolino force-pushed the kubernetes-1.12 branch from 62895e4 to 6f74d7a Compare January 8, 2019 15:30

cynepco3hahue reviewed Jan 9, 2019

View reviewed changes

rmohr requested changes Jan 9, 2019

View reviewed changes

gonzolino force-pushed the kubernetes-1.12 branch from 6f74d7a to dd4bb6c Compare January 9, 2019 18:21

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2019

slintes suggested changes Jan 14, 2019

View reviewed changes

gonzolino force-pushed the kubernetes-1.12 branch from 3a698bc to 7753561 Compare January 14, 2019 23:37

slintes approved these changes Jan 15, 2019

View reviewed changes

slintes approved these changes Jan 16, 2019

View reviewed changes

davidvossel requested changes Jan 16, 2019

View reviewed changes

pkg/virt-controller/watch/application.go Outdated Show resolved Hide resolved

tools/util/marshaller.go Show resolved Hide resolved

gonzolino force-pushed the kubernetes-1.12 branch from 7753561 to 2b1ccc3 Compare January 16, 2019 20:22

davidvossel approved these changes Jan 17, 2019

View reviewed changes

gonzolino and others added 5 commits January 21, 2019 12:03

Update dependencies to kubernetes 1.12.4

7b8685f

Introduce a Context to virt-controller

5914d69

Override required dataSource for PersistentVolumeClaimSpec

0d229fc

Remove required attribute from openAPI and remove empty dataSource fields in marshaller See kubevirt#1882 (comment) for more details.

Regenerate code

a4d5b97

fix merge conflicts and re-run 'make generate'

97bae30

Signed-off-by: David Vossel <davidvossel@gmail.com>

davidvossel force-pushed the kubernetes-1.12 branch from 2b1ccc3 to 97bae30 Compare January 21, 2019 17:38

cynepco3hahue merged commit 2f1714d into kubevirt:master Jan 23, 2019

Update to kubernetes 1.12.4 #1882

Update to kubernetes 1.12.4 #1882

Conversation

gonzolino commented Jan 2, 2019

ovirt-infra commented Jan 2, 2019

cynepco3hahue commented Jan 2, 2019

gonzolino commented Jan 2, 2019

cynepco3hahue commented Jan 2, 2019

fabiand commented Jan 4, 2019

cynepco3hahue commented Jan 6, 2019

rmohr left a comment

Choose a reason for hiding this comment

mfranczy commented Jan 7, 2019 • edited

mfranczy commented Jan 7, 2019 • edited

rmohr commented Jan 7, 2019

mfranczy commented Jan 7, 2019

cynepco3hahue commented Jan 7, 2019

gonzolino commented Jan 7, 2019

cynepco3hahue commented Jan 7, 2019

cynepco3hahue commented Jan 8, 2019

cynepco3hahue commented Jan 9, 2019

cynepco3hahue Jan 9, 2019

Choose a reason for hiding this comment

mfranczy Jan 9, 2019

Choose a reason for hiding this comment

gonzolino Jan 9, 2019

Choose a reason for hiding this comment

rmohr Jan 9, 2019

Choose a reason for hiding this comment

mfranczy commented Jan 9, 2019

rmohr left a comment

Choose a reason for hiding this comment

mfranczy commented Jan 9, 2019

slintes commented Jan 14, 2019

slintes left a comment

Choose a reason for hiding this comment

slintes Jan 14, 2019

Choose a reason for hiding this comment

gonzolino Jan 14, 2019

Choose a reason for hiding this comment

gonzolino commented Jan 15, 2019

slintes left a comment

Choose a reason for hiding this comment

slintes Jan 15, 2019

Choose a reason for hiding this comment

slintes commented Jan 15, 2019

slintes left a comment

Choose a reason for hiding this comment

davidvossel left a comment

Choose a reason for hiding this comment

davidvossel left a comment

Choose a reason for hiding this comment

davidvossel commented Jan 17, 2019

davidvossel commented Jan 18, 2019

davidvossel commented Jan 21, 2019

davidvossel commented Jan 21, 2019

davidvossel commented Jan 21, 2019

cynepco3hahue commented Jan 22, 2019

cynepco3hahue commented Jan 22, 2019

cynepco3hahue commented Jan 22, 2019

davidvossel commented Jan 22, 2019

cynepco3hahue commented Jan 23, 2019

mfranczy commented Jan 7, 2019 •

edited

mfranczy commented Jan 7, 2019 •

edited