Skip to content

Bug 1825323: replace terraform-provider-vsphere with OpenShift fork#3603

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
patrickdillon:vsphere-vsan-bz-1825323
May 20, 2020
Merged

Bug 1825323: replace terraform-provider-vsphere with OpenShift fork#3603
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
patrickdillon:vsphere-vsan-bz-1825323

Conversation

@patrickdillon
Copy link
Copy Markdown
Contributor

@patrickdillon patrickdillon commented May 14, 2020

This PR:

  1. switches the terraform-provider-vsphere to the OpenShift fork. The commit message has details on the motivation.
  2. Updates the vsphereprivate based on the newly vendored code.

/hold
this currently points to my fork as a POC
Depends on openshift/terraform-provider-vsphere#2

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 14, 2020
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented May 14, 2020

@patrickdillon: This pull request references Bugzilla bug 1825323, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1825323: replace terraform-provider-vsphere with OpenShift fork [WIP]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 14, 2020
@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 14, 2020

/test e2e-vsphere

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 14, 2020

/test e2e-vsphere-upi

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 14, 2020

cc @jcpowermac especially please take a look at the update to vsphereprivate

@abhinavdahiya
Copy link
Copy Markdown
Contributor

abhinavdahiya commented May 14, 2020

/test e2e-vsphere

@jcpowermac
Copy link
Copy Markdown
Contributor

jcpowermac commented May 14, 2020

cc @jcpowermac especially please take a look at the update to vsphereprivate

Looks good. Same general changes I had to make to test w/1.18.x

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 14, 2020

/test e2e-vsphere

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 14, 2020

local testing with vsan is provisioning thin-disks and completing bootstrap. cluster is failing with:

E0514 14:43:04.346444  380190 reflector.go:307] k8s.io/client-go/tools/watch/informerwatcher.go:146: Failed to watch *v1.ClusterVersion: Get "https://api.padillon.devcluster.openshift.local:6443/apis/config.openshift.io/v1/clusterversions?allowWatchBookmarks=true&fieldSelector=metadata.name%3Dversion&resourceVersion=16576&timeoutSeconds=348&watch=true": dial tcp 172.29.0.31:6443: connect: connection refused

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 15, 2020

/test e2e-vsphere

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 15, 2020

local testing succeeded. I had messed up my install-config while adjusting to working in new VMC env.

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 15, 2020

With successful testing both masters & workers are being successfully provisioned with thin-disks. This resolves the BZ.

@patrickdillon patrickdillon mentioned this pull request May 15, 2020
Merged
@patrickdillon
vendor: replace terraform-provider-vsphere with OpenShift fork
e5e94e0 
With https://bugzilla.redhat.com/show_bug.cgi?id=1825323 we see terraform
failing with an inconsistent plan when the vSAN storage policy enforces
thin-provisioned disks but for some reason the terraform provider produces
a plan stating they are thick-provisioned. Terraform docs indicates these
types of problems require a fix to CustomizeDiff in the provider:
https://www.terraform.io/docs/extend/terraform-0.12-compatibility.html#inaccurate-plans

In a recent PR
vmware/terraform-provider-vsphere#1052
we see that when the VM is being imported from an OVA, the DiskDiffOperation
is simply skipped. DiskDiffOperation is the function that produces the
inconsistent plan in the bugzilla mentioned before.  We have created a
similar PR to skip DiskDiffOperation when VMs are being cloned:
vmware/terraform-provider-vsphere#1075

This commit carries that patch until it has merged upstream or resolved otherwise

The commit was created by running:
 go mod edit -replace github.com/terraform-providers/terraform-provider-vsphere=github.com/openshift/terraform-provider-vsphere@v1.18.1-openshift-1
 go mod edit -replace github.com/vmware/govmomi=github.com/vmware/govmomi@v0.22.2-0.20200420222347-5fceac570f29
 go mod tidy
 go mod vendor

This commit also updates the vsphereprivate provider to use the updated libraries.
@patrickdillon patrickdillon force-pushed the vsphere-vsan-bz-1825323 branch from 44f7428 to e5e94e0 Compare May 19, 2020 15:31
@patrickdillon patrickdillon changed the title Bug 1825323: replace terraform-provider-vsphere with OpenShift fork [WIP] Bug 1825323: replace terraform-provider-vsphere with OpenShift fork May 19, 2020
@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 19, 2020

I have updated this PR now that openshift/terraform-provider-vsphere#2 has merged and have removed the WIP. I think all work is done unless testing fails.

/test e2e-vsphere
I think we can cancel the hold if install succeeds

@patrickdillon
Copy link
Copy Markdown
Contributor Author

patrickdillon commented May 19, 2020

Install succeeded in e2e-vsphere so I am comfortable to merge.
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 19, 2020
@abhinavdahiya
Copy link
Copy Markdown
Contributor

abhinavdahiya commented May 19, 2020

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 19, 2020
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented May 19, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2020
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 20, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 20, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 20, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 20, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 20, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented May 20, 2020

@patrickdillon: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 e5e94e0 link /test e2e-aws-scaleup-rhel7
ci/prow/e2e-vsphere e5e94e0 link /test e2e-vsphere

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 2dc5c62 into openshift:master May 20, 2020
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented May 20, 2020

@patrickdillon: Some pull requests linked via external trackers have merged: openshift/installer#3603. The following pull requests linked via external trackers have not merged:

Details

In response to this:

Bug 1825323: replace terraform-provider-vsphere with OpenShift fork

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabianofranz fabianofranz Awaiting requested review from fabianofranz

@jstuever jstuever Awaiting requested review from jstuever

Assignees

@abhinavdahiya abhinavdahiya

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@patrickdillon @openshift-ci-robot @abhinavdahiya @jcpowermac @openshift-bot @openshift-merge-robot