*: Make libvirt support completely conditional (behind TAGS=libvirt) #955

wking · 2018-12-20T05:41:36Z

Previously, destroy support was behind TAGS=libvirt_destroy and create support was always built in. But since 3fb4400 (#919), the bundled libvirt Terraform provider has also been behind libvirt_destroy. That leads to cluster creation failing with:

$ openshift-install create cluster
...
ERROR Missing required providers.
ERROR
ERROR The following provider constraints are not met by the currently-installed
ERROR provider plugins:
ERROR
ERROR * libvirt (any version)
ERROR
ERROR Terraform can automatically download and install plugins to meet the given
ERROR constraints, but this step was skipped due to the use of -get-plugins=false
ERROR and/or -plugin-dir on the command line.
...

With this commit, folks trying to create cluster without libvirt compiled in will get:

FATAL failed to fetch Common Manifests: failed to load asset "Install Config": invalid "install-config.yaml" file: platform: Invalid value: types.Platform{AWS:(*aws.Platform)(nil), Libvirt:(*libvirt.Platform)(0xc4209511f0), OpenStack:(*openstack.Platform)(nil)}: platform must be one of: aws, openstack

before we get to Terraform.

Now that the build tag guards both creation and deletion, I've renamed it from libvirt_destroy to the unqualified libvirt.

CC @abhinavdahiya

pkg/types/validation/installconfig.go

michaelgugino

BYO needs libvirt support by default. We should always bundle in libvirt, this patch set should be going the otherway.

crawford · 2018-12-20T19:49:45Z

We discussed this earlier and decided to add a vanilla platform. This can be used by BYOR.

Previously, destroy support was behind TAGS=libvirt_destroy and create support was always built in. But since 3fb4400 (terraform/plugins: add `libvirt`, `aws`, `ignition`, `openstack` to KnownPlugins, 2018-12-14, openshift#919), the bundled libvirt Terraform provider has also been behind libvirt_destroy. That leads to cluster creation failing with: $ openshift-install create cluster ... ERROR Missing required providers. ERROR ERROR The following provider constraints are not met by the currently-installed ERROR provider plugins: ERROR ERROR * libvirt (any version) ERROR ERROR Terraform can automatically download and install plugins to meet the given ERROR constraints, but this step was skipped due to the use of -get-plugins=false ERROR and/or -plugin-dir on the command line. ... With this commit, folks trying to 'create cluster' without libvirt compiled in will get: FATAL failed to fetch Common Manifests: failed to load asset "Install Config": invalid "install-config.yaml" file: platform: Invalid value: types.Platform{AWS:(*aws.Platform)(nil), Libvirt:(*libvirt.Platform)(0xc4209511f0), OpenStack:(*openstack.Platform)(nil)}: platform must be one of: aws, openstack before we get to Terraform. Now that the build tag guards both creation and deletion, I've renamed it from 'libvirt_destroy' to the unqualified 'libvirt'. I've also adjusted the install-config validation testing to use regular expressions so we can distinguish between failures because libvirt was not compiled in as a valid platform and failures because some portion of the libvirt configuration was broken. In order to get stable error messages for comparison, I've added some strings.Sort calls for various allowed-value string-slice computations.

pkg/types/validation/installconfig_test.go

wking · 2018-12-22T00:29:37Z

e2e-aws included:

fail [github.com/openshift/origin/test/extended/operators/cluster.go:109]: Expected <[]string | len:1, cap:1>: [ "Pod openshift-cluster-machine-approver/machine-approver-649d8ffd59-t5fjm is not healthy: container machine-approver-controller has restarted more than 5 times", ] to be empty

Must be unrelated.

/retest

wking · 2018-12-22T00:57:54Z

Digging into the machine-approver restarts:

$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/955/pull-ci-openshift-installer-master-e2e-aws/2493/artifacts/e2e-aws/events.json | jq '.items[] | select(.involvedObject.namespace == "openshift-cluster-machine-approver" and .message == "Back-off restarting failed container")'
{
  "apiVersion": "v1",
  "count": 17,
  "eventTime": null,
  "firstTimestamp": "2018-12-21T21:35:41Z",
  "involvedObject": {
    "apiVersion": "v1",
    "fieldPath": "spec.containers{machine-approver-controller}",
    "kind": "Pod",
    "name": "machine-approver-649d8ffd59-t5fjm",
    "namespace": "openshift-cluster-machine-approver",
    "resourceVersion": "1599",
    "uid": "11c9551a-0568-11e9-a8a1-122c3fd60c36"
  },
  "kind": "Event",
  "lastTimestamp": "2018-12-21T21:42:26Z",
  "message": "Back-off restarting failed container",
  "metadata": {
    "creationTimestamp": "2018-12-21T21:35:41Z",
    "name": "machine-approver-649d8ffd59-t5fjm.157276b502925b15",
    "namespace": "openshift-cluster-machine-approver",
    "resourceVersion": "10209",
    "selfLink": "/api/v1/namespaces/openshift-cluster-machine-approver/events/machine-approver-649d8ffd59-t5fjm.157276b502925b15",
    "uid": "5dc520a3-0568-11e9-a8a1-122c3fd60c36"
  },
  "reason": "BackOff",
  "reportingComponent": "",
  "reportingInstance": "",
  "source": {
    "component": "kubelet",
    "host": "ip-10-0-23-128.ec2.internal"
  },
  "type": "Warning"
}

The pod logs suggest the issue may have been a missing Kubernetes API server:

$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/955/pull-ci-openshift-installer-master-e2e-aws/2493/artifacts/e2e-aws/pods/openshift-cluster-machine-approver_machine-approver-649d8ffd59-t5fjm_machine-approver-controller_previous.log.gz | gunzip | tail
E1221 21:42:24.392324       1 leaderelection.go:234] error retrieving resource lock openshift-cluster-machine-approver/cluster-machine-approver-lock: Get https://127.0.0.1:6443/api/v1/namespaces/openshift-cluster-machine-approver/configmaps/cluster-machine-approver-lock: dial tcp 127.0.0.1:6443: connect: connection refused
I1221 21:42:25.279413       1 reflector.go:240] Listing and watching *v1beta1.CertificateSigningRequest from github.com/openshift/cluster-machine-approver/vendor/k8s.io/client-go/informers/factory.go:130
E1221 21:42:25.279885       1 reflector.go:205] github.com/openshift/cluster-machine-approver/vendor/k8s.io/client-go/informers/factory.go:130: Failed to list *v1beta1.CertificateSigningRequest: Get https://127.0.0.1:6443/apis/certificates.k8s.io/v1beta1/certificatesigningrequests?limit=500&resourceVersion=0: dial tcp 127.0.0.1:6443: connect: connection refused
I1221 21:42:26.280099       1 reflector.go:240] Listing and watching *v1beta1.CertificateSigningRequest from github.com/openshift/cluster-machine-approver/vendor/k8s.io/client-go/informers/factory.go:130
E1221 21:42:26.280507       1 reflector.go:205] github.com/openshift/cluster-machine-approver/vendor/k8s.io/client-go/informers/factory.go:130: Failed to list *v1beta1.CertificateSigningRequest: Get https://127.0.0.1:6443/apis/certificates.k8s.io/v1beta1/certificatesigningrequests?limit=500&resourceVersion=0: dial tcp 127.0.0.1:6443: connect: connection refused
E1221 21:42:26.392153       1 leaderelection.go:234] error retrieving resource lock openshift-cluster-machine-approver/cluster-machine-approver-lock: Get https://127.0.0.1:6443/api/v1/namespaces/openshift-cluster-machine-approver/configmaps/cluster-machine-approver-lock: dial tcp 127.0.0.1:6443: connect: connection refused
E1221 21:42:26.392401       1 leaderelection.go:234] error retrieving resource lock openshift-cluster-machine-approver/cluster-machine-approver-lock: Get https://127.0.0.1:6443/api/v1/namespaces/openshift-cluster-machine-approver/configmaps/cluster-machine-approver-lock: dial tcp 127.0.0.1:6443: connect: connection refused
E1221 21:42:26.392463       1 event.go:259] Could not construct reference to: '&v1.ConfigMap{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:v1.ObjectMeta{Name:"", GenerateName:"", Namespace:"", SelfLink:"", UID:"", ResourceVersion:"", Generation:0, CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}, DeletionTimestamp:(*v1.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Initializers:(*v1.Initializers)(nil), Finalizers:[]string(nil), ClusterName:""}, Data:map[string]string(nil), BinaryData:map[string][]uint8(nil)}' due to: 'selfLink was empty, can't make reference'. Will not report event: 'Normal' 'LeaderElection' 'd6a1f746-0568-11e9-b47d-0a9f534285c0 stopped leading'
I1221 21:42:26.392532       1 leaderelection.go:213] failed to renew lease openshift-cluster-machine-approver/cluster-machine-approver-lock: timed out waiting for the condition
F1221 21:42:26.392554       1 leaderelection.go:65] leaderelection lost

which matches the event's lastTimestamp. But it's hard to imaging the Kubernetes API server being down for the full seven minutes we were seeing that event without it affecting other parts of the e2e tests.

wking · 2018-12-22T02:17:22Z

All green :)

crawford · 2018-12-22T02:25:05Z

/lgtm

openshift-ci-robot · 2018-12-22T02:25:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [crawford,wking]

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 added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 20, 2018

openshift-ci-robot requested review from flaper87 and russellb December 20, 2018 05:46

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2018

staebler reviewed Dec 20, 2018

View reviewed changes

pkg/types/validation/installconfig.go Outdated Show resolved Hide resolved

michaelgugino suggested changes Dec 20, 2018

View reviewed changes

This was referenced Dec 20, 2018

add libvirt tag to libvirt-ci Dockerfile #961

Merged

fixup/update installer-libvirt template openshift/release#2410

Merged

wking force-pushed the decouple-libvirt branch from 496b536 to 432f283 Compare December 21, 2018 21:11

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 21, 2018

staebler reviewed Dec 21, 2018

View reviewed changes

pkg/types/validation/installconfig_test.go Show resolved Hide resolved

wking mentioned this pull request Dec 22, 2018

Revert "pkg/asset/machines/aws/machinesets: Give workers public IPs (for now)" #970

Merged

openshift-ci-robot assigned crawford Dec 22, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2018

openshift-merge-robot merged commit a29ed8f into openshift:master Dec 22, 2018

wking mentioned this pull request Dec 22, 2018

Revert "Operator restructure" openshift/cluster-machine-approver#9

Merged

wking deleted the decouple-libvirt branch December 22, 2018 04:32

vrutkovs mentioned this pull request Jan 2, 2019

Add an installer image for libvirt #978

Closed

abhinavdahiya mentioned this pull request Jan 2, 2019

v0.8 release does not show libvirt in platforms #979

Closed

vrutkovs mentioned this pull request Jan 2, 2019

WIP Run BYOR install on installer presubmits openshift/release#2481

Closed

wking mentioned this pull request Feb 8, 2019

Provide builds with libvirt support #1213

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: Make libvirt support completely conditional (behind TAGS=libvirt) #955

*: Make libvirt support completely conditional (behind TAGS=libvirt) #955

wking commented Dec 20, 2018

michaelgugino left a comment

crawford commented Dec 20, 2018

wking commented Dec 22, 2018

wking commented Dec 22, 2018

wking commented Dec 22, 2018

crawford commented Dec 22, 2018

openshift-ci-robot commented Dec 22, 2018

*: Make libvirt support completely conditional (behind TAGS=libvirt) #955

*: Make libvirt support completely conditional (behind TAGS=libvirt) #955

Conversation

wking commented Dec 20, 2018

michaelgugino left a comment

Choose a reason for hiding this comment

crawford commented Dec 20, 2018

wking commented Dec 22, 2018

wking commented Dec 22, 2018

wking commented Dec 22, 2018

crawford commented Dec 22, 2018

openshift-ci-robot commented Dec 22, 2018