Bug 1735192: OpenStack Self Signed Certs Fix by iamemilio · Pull Request #2587 · openshift/installer

iamemilio · 2019-10-29T18:14:38Z

Create Bootstrap Ignition shim 100% in GO code in order to use the latest versions of ignition.

openshift-ci-robot · 2019-10-29T18:14:42Z

@iamemilio: This pull request references Bugzilla bug 1735192, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

[WIP] Bug 1735192: OpenStack Self Signed Certs Fix

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.

iamemilio · 2019-10-29T18:14:52Z

/label platform/openstack

abhinavdahiya · 2019-10-29T19:09:42Z

try moving these out to a separate file.

Will do, its getting ugly in there

iamemilio · 2019-10-29T22:26:32Z

/retest

iamemilio · 2019-10-30T15:37:56Z

Deployed successfully on a cluster with Self Signed Certs, ready to merge!

iamemilio · 2019-10-30T15:43:35Z

/test e2e-aws-scaleup-rhel7

Fedosin · 2019-10-30T16:03:20Z

I think these resources (container and object) won't be destroyed by the installer, since they don't belong to Terraform anymore: https://github.com/openshift/installer/blob/master/pkg/destroy/bootstrap/bootstrap.go#L69
We need to add some logic to explicitly destroy them when bootstrapping is over.

Fedosin · 2019-10-30T16:07:25Z

Previously we used a string of 16 characters to generate the url, now it's just a number. I recommend to use https://github.com/kubernetes/apimachinery/blob/master/pkg/util/rand/rand.go#L98 to generate a random string.

Add dependancies for gophercloud objectstorage

iamemilio · 2019-10-30T19:48:05Z

/retest

Fedosin · 2019-10-30T23:04:50Z

It would be nice if we can move this function to pkg/destroy/bootstrap/bootstrap.go and use it there directly, but I don't mind to keep it here.

I originally put it in a new directory pkg/destroy/bootstrap/openstack/swift.go it seemed redundant. I wasnt sure if @abhinavdahiya would take issue with openstack functions being in pkg/destroy/bootstrap/bootstrap.go. @abhinavdahiya do you have a preference?

Fedosin · 2019-10-30T23:10:30Z

/lgtm

However, adding a few comments on how it would work would be nice.

iamemilio · 2019-10-31T00:06:39Z

/test e2e-aws-scaleup-rhel7

Fedosin · 2019-10-31T00:27:45Z

e2e-aws-scaleup-rhel7 is broken, no need to retest it

tomassedovic · 2019-10-31T14:09:23Z

/lgtm

Adding mine too. @abhinavdahiya any chance we could get an approve before the feature freeze please?

abhinavdahiya · 2019-10-31T16:40:26Z

why is this import separate from other installer repo imports.

abhinavdahiya · 2019-10-31T16:40:36Z

use errors.Wrapf

abhinavdahiya · 2019-10-31T16:41:04Z

This function should be private

abhinavdahiya · 2019-10-31T16:41:19Z

this function should also be private

iamemilio · 2019-10-31T18:01:07Z

/test e2e-openstack

abhinavdahiya · 2019-10-31T18:10:21Z

metadata.ClusterID -> metadata.InfraID

abhinavdahiya · 2019-10-31T18:29:48Z

/approve

openshift-ci-robot · 2019-10-31T19:15:20Z

@iamemilio: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-scaleup-rhel7	`9171ae3`	link	`/test e2e-aws-scaleup-rhel7`

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.

Fedosin · 2019-10-31T19:57:08Z

/lgtm

openshift-ci-robot · 2019-10-31T19:57:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, Fedosin, iamemilio, tomassedovic

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:

~~OWNERS~~ [abhinavdahiya]

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 · 2019-10-31T22:23:13Z

@iamemilio: All pull requests linked via external trackers have merged. Bugzilla bug 1735192 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1735192: OpenStack Self Signed Certs Fix

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 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 29, 2019

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 29, 2019

openshift-ci-robot requested review from jhixson74 and jstuever October 29, 2019 18:14

openshift-ci-robot added the platform/openstack label Oct 29, 2019

abhinavdahiya reviewed Oct 29, 2019

View reviewed changes

iamemilio force-pushed the self-signed-certs branch from b8cb54e to 6a2370e Compare October 29, 2019 21:21

iamemilio changed the title ~~[WIP] Bug 1735192: OpenStack Self Signed Certs Fix~~ Bug 1735192: OpenStack Self Signed Certs Fix Oct 30, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 30, 2019

Fedosin suggested changes Oct 30, 2019

View reviewed changes

iamemilio force-pushed the self-signed-certs branch from 6a2370e to e0e7cc0 Compare October 30, 2019 16:51

Fedosin and others added 2 commits October 30, 2019 13:14

OpenStack: support self-signed certificates

63f7a03

Add dependancies for gophercloud objectstorage

Update Dependancies: Gophercloud Objectstore, Kubernetes rand

d15c43e

iamemilio force-pushed the self-signed-certs branch 3 times, most recently from 6a98283 to 2d3b1e3 Compare October 30, 2019 18:39

Fedosin reviewed Oct 30, 2019

View reviewed changes

openshift-ci-robot assigned Fedosin Oct 30, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2019

openshift-ci-robot assigned tomassedovic Oct 31, 2019

abhinavdahiya reviewed Oct 31, 2019

View reviewed changes

Comment thread pkg/destroy/bootstrap/bootstrap.go Outdated

Copy link
Copy Markdown

Contributor

abhinavdahiya Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use errors.Wrapf

abhinavdahiya reviewed Oct 31, 2019

View reviewed changes

Comment thread pkg/tfvars/openstack/bootstrap_ignition.go Outdated

Copy link
Copy Markdown

Contributor

abhinavdahiya Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should be private

abhinavdahiya reviewed Oct 31, 2019

View reviewed changes

Comment thread pkg/tfvars/openstack/bootstrap_ignition.go Outdated

Copy link
Copy Markdown

Contributor

abhinavdahiya Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function should also be private

iamemilio force-pushed the self-signed-certs branch from 2d3b1e3 to a273c47 Compare October 31, 2019 16:55

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2019

iamemilio force-pushed the self-signed-certs branch from a273c47 to d02c55e Compare October 31, 2019 16:57

abhinavdahiya reviewed Oct 31, 2019

View reviewed changes

support delete during bootstrap complete

9171ae3

iamemilio force-pushed the self-signed-certs branch from d02c55e to 9171ae3 Compare October 31, 2019 18:19

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 31, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2019

openshift-merge-robot merged commit efddb40 into openshift:master Oct 31, 2019

abhinavdahiya mentioned this pull request Nov 5, 2019

tf ignition provider does not support the use of a custom CA #2225

Closed

russellb mentioned this pull request Aug 13, 2020

Bootstrap Ignition Config: provisional state openshift/enhancements#72

Closed

Conversation

iamemilio commented Oct 29, 2019

Uh oh!

openshift-ci-robot commented Oct 29, 2019

Uh oh!

iamemilio commented Oct 29, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamemilio commented Oct 29, 2019

Uh oh!

iamemilio commented Oct 30, 2019

Uh oh!

iamemilio commented Oct 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamemilio commented Oct 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fedosin commented Oct 30, 2019

Uh oh!

iamemilio commented Oct 31, 2019

Uh oh!

Fedosin commented Oct 31, 2019

Uh oh!

tomassedovic commented Oct 31, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamemilio commented Oct 31, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhinavdahiya commented Oct 31, 2019

Uh oh!

openshift-ci-robot commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fedosin commented Oct 31, 2019

Uh oh!

openshift-ci-robot commented Oct 31, 2019

Uh oh!

openshift-ci-robot commented Oct 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

openshift-ci-robot commented Oct 31, 2019 •

edited

Loading