Skip to content
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

Bug 1831711: Fix incompatible GVK detection for 4.3->4.4 migrations #530

Merged
merged 5 commits into from May 11, 2020

Conversation

alaypatel07
Copy link
Contributor

@alaypatel07 alaypatel07 commented May 8, 2020

This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=1831711.

The controller now uses ServerPreferredNamespacedResources to
list the preferred namespaced resources on source cluster and then
compare the group version with destination cluster. It adds two unit
tests to verify. As further steps, more extensive unit test needs to
be added to understand this better

@mig-ci-robot
Copy link

Can one of the admins verify this patch?

@djwhatle djwhatle marked this pull request as draft May 8, 2020 20:31
@djwhatle djwhatle changed the title [DO NOT MERGE]: add unit test for comparing deployments CRDs for 4.3->4.4 migrations Add unit test for comparing deployments CRDs for 4.3->4.4 migrations May 8, 2020
@djwhatle djwhatle added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label May 8, 2020
@alaypatel07
Copy link
Contributor Author

Logging the initial PR comment for history


This PR is meant to help in figuring out the problem for https://bugzilla.redhat.com/show_bug.cgi?id=1831711

This unit test needs to pass. My understanding is that in 4.4 only `apps/v1` is supported for Deployment. But the controllers checks if `apps/v1beta2` exists in 4.4 as it exists in 4.2

Probably using `ServerPreferredNamespacedResources`[1] for collecting source GVK's[2][3] might fix this bug.

1. https://github.com/konveyor/mig-controller/blob/e333d47d55dc6317e4a0495fafbb6cb991445204/vendor/k8s.io/client-go/discovery/discovery_client.go#L93
2. https://github.com/konveyor/mig-controller/blob/b3398c28b68679af5b6f912e02373c517c0a2d82/pkg/gvk/gvk.go#L33
3. https://github.com/konveyor/mig-controller/blob/b3398c28b68679af5b6f912e02373c517c0a2d82/pkg/gvk/gvk.go#L59

@djwhatle djwhatle self-requested a review May 8, 2020 22:22
@djwhatle djwhatle marked this pull request as ready for review May 8, 2020 22:23
@djwhatle djwhatle removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label May 8, 2020
@djwhatle
Copy link
Contributor

djwhatle commented May 8, 2020

\test

@djwhatle djwhatle changed the title Add unit test for comparing deployments CRDs for 4.3->4.4 migrations Fix incompatible GVK detection for 4.3->4.4 migrations May 8, 2020
@djwhatle djwhatle added this to the 1.2.0 milestone May 8, 2020
@djwhatle djwhatle changed the title Fix incompatible GVK detection for 4.3->4.4 migrations Bug 1831711: Fix incompatible GVK detection for 4.3->4.4 migrations May 8, 2020
@djwhatle djwhatle added the kind/bug Categorizes issue or PR as related to a bug. label May 8, 2020
@mig-ci-robot
Copy link

✔️ Setup e2e environment
✔️ Login to OCP 3.7
✔️ Login to OCP 4.3
✔️ Build mig-controller image
✔️ Deploy mig-operator on source cluster
✔️ Deploy mig-controller on source cluster
✔️ Deploy mig-operator on destination cluster
✔️ Deploy mig-controller on destination cluster
✔️ Execute migration with test cases : all

Find full build log here
Find instructions to debug here

@djwhatle
Copy link
Contributor

This PR narrows the GVK diff list so that migrations 4.3 -> 4.4 can run. In a followup PR we plan to only notify the user of a GVK diff if the GVK diff is present in a namespace selected for migration.

@djwhatle djwhatle merged commit 4a92b21 into migtools:master May 11, 2020
djwhatle pushed a commit that referenced this pull request May 11, 2020
…530)

* add unit test for comparing deployments CRDs for 4.3->4.4 migrations

* add failing test for collectResources

* add collectPreferredResources and use it to collect sources GVR's

* fix unit test for compareResources

* update vendor

(cherry picked from commit 4a92b21)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. release-1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants