Skip to content

Conversation

@smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Jan 22, 2019

This is a prototype to assist in documenting how someone would add a new platform to OpenShift.

STATUS: Currently blocked because MCO panics with no such platform

This wires a minimum of create logic and behavior to enable a new platform 'google' (name should be 'gcp', but terraform fails when platform name != terraform-cloud-provider name). The destroy function is wired directly to terraform for now and is considered a bare minimum step.

The terraform steps are consistent with AWS and the 3.11 provisioning playbooks in openshift-ansible - roughly:

  1. Create bootstrap in zone[1], masters in zones[1..3]
  2. Use internal network load balancer for masters
  3. Use TCP proxy for external load balancer
  4. (no domain name setup yet)
  5. Directly inject bootstrap.ign to user-data since we can on GCP
  6. Using a hardcoded maipo image since we don't have a continuous run
  7. Create a service account for the bootstrap
  8. Wire through instance size, disk, and disk size

TODO:

This wires a minimum of create logic and behavior to enable a new
platform 'google' (name should be 'gcp', but terraform fails when
platform name != terraform-cloud-provider name). The destroy function
is wired directly to terraform for now and is considered a bare
minimum step.

The terraform steps are consistent with AWS and the 3.11 provisioning
playbooks in openshift-ansible - roughly:

1. Create bootstrap in zone[1], masters in zones[1..3]
2. Use internal network load balancer for masters
3. Use TCP proxy for external load balancer
4. (no domain name setup yet)
5. Directly inject bootstrap.ign to user-data since we can on GCP
6. Using a hardcoded maipo image since we don't have a continuous run
7. Create a service account for the bootstrap
8. Wire through instance size, disk, and disk size
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 22, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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 Jan 22, 2019
@russellb
Copy link
Contributor

@flaper87 Didn't you write some docs tracking everything that was needed for OpenStack that could possibly help with the documentation goal written here?

@smarterclayton
Copy link
Contributor Author

please comment on #1112

err := survey.Ask([]*survey.Question{
{
Prompt: &survey.Input{
Message: "AWS Access Key ID",
Copy link
Member

Choose a reason for hiding this comment

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

Some AWS leftovers here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I don't take credentials yet. Once we settle on behavior of the various operators on unrecognized clouds I'm going to go open a bunch of PRs /bugs and then push through more.

user-data = "${var.ignition}"
}

tags = ["ocp", "ocp-master"]

Choose a reason for hiding this comment

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

I recommend adding the the ability for a user to add additional tags to this list. This will be necessary for environments with restrictive GCP firewalls that require specific tags to be set to allow network communication.


network_interface = {
network = "${var.subnetwork != "" ? "" : var.network}"
subnetwork = "${var.subnetwork}"

Choose a reason for hiding this comment

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

Some environments may require using a GCP Shared VPC. The above code will not work with a Shared VPC.

@@ -0,0 +1,75 @@
resource "google_compute_instance" "master" {

Choose a reason for hiding this comment

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

You might want to consider adding a scheduling block to the master GCE instance. Something like this ...

scheduling {
    automatic_restart = "true"
    on_host_maintenance = "MIGRATE"
    preemptible = "false"
  }

By default the Terraform Google Cloud Provider will not enable automatic_restart if you don't explicitly set it. Looks like this will be be fixed in version 2.0 of the provider. See hashicorp/terraform-provider-google#2638 for details.


network_interface = {
network = "${var.subnetwork != "" ? "" : var.network}"
subnetwork = "${var.subnetwork}"

Choose a reason for hiding this comment

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

Similar comment as above regarding GCP Shared VPC. Some users may require using a Shared VPC.

health_checks = ["${google_compute_health_check.masters.self_link}"]
}

resource "google_compute_target_tcp_proxy" "master" {

Choose a reason for hiding this comment

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

It looks like this load balancer is Internet facing. Do you have plans to support a GCP ILB at some point in the future?

https://cloud.google.com/load-balancing/docs/internal/

@@ -0,0 +1,163 @@
// Package aws collects AWS-specific configuration.

Choose a reason for hiding this comment

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

Maybe change this to // Package google collects GCP-specfic configuration.

Logger logrus.FieldLogger
}

// New returns an OpenStack destroyer from ClusterMetadata.

Choose a reason for hiding this comment

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

s/OpenStack/GCP/

)

var (
// Regions is a map of the known AWS regions. The key of the map is

Choose a reason for hiding this comment

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

s/AWS/GCP/

// the short name of the region. The value of the map is the long
// name of the region.
Regions = map[string]string{
"us-central1": "Ohio",

Choose a reason for hiding this comment

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

Probably doesn't matter, but us-central1 is in Iowa.

@openshift-ci-robot
Copy link
Contributor

@smarterclayton: PR needs rebase.

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.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2019
@openshift-ci-robot
Copy link
Contributor

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

Test name Commit Details Rerun command
ci/prow/tf-fmt 3c4e92e link /test tf-fmt
ci/prow/golint 3c4e92e link /test golint
ci/prow/e2e-aws-upgrade 3c4e92e link /test e2e-aws-upgrade
ci/prow/verify-vendor 3c4e92e link /test verify-vendor
ci/prow/e2e-aws 3c4e92e link /test e2e-aws

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.

@abhinavdahiya
Copy link
Contributor

IPI for GCP was implemented using this. and this is now obsolete.

/close

@openshift-ci-robot
Copy link
Contributor

@abhinavdahiya: Closed this PR.

Details

In response to this:

IPI for GCP was implemented using this. and this is now obsolete.

/close

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants