Remove GCP cluster api provider #5437

patrickdillon · 2021-12-01T17:14:06Z

I ended up doing this work while trying to eliminate our need to replace the cluster-api-provider-azure. More work is needed beyond this to achieve that, but we might as well commit it. I am doing this work for Azure, but we should be able to do something similar for all other providers that have migrated to openshift/api.

patrickdillon · 2021-12-01T17:14:34Z

/cc @kirankt @staebler

patrickdillon · 2021-12-01T17:15:03Z

/test e2e-gcp

I thought these tests should be run automatically with recent changes, but it looks like they are not getting queued.

patrickdillon · 2021-12-01T17:17:42Z

/test e2e-gcp

kirankt

Thanks @patrickdillon I actaully meant to work on this today as well. Left a few comments. PTAL.

kirankt · 2021-12-01T17:55:41Z

pkg/asset/machines/master.go

@@ -537,7 +535,6 @@ func (m *Master) Machines() ([]machineapi.Machine, error) {
 	awsapi.AddToScheme(scheme)
 	azureapi.AddToScheme(scheme)
 	baremetalapi.AddToScheme(scheme)
-	gcpapi.AddToScheme(scheme)


I think we will need to add the GCP types into the schema here.
e.g.

installer/pkg/asset/machines/master.go

Lines 545 to 548 in c3a4dbc

// Add vsphere types to scheme

scheme.AddKnownTypes(machineapi.SchemeGroupVersion,

&machineapi.VSphereMachineProviderSpec{},

)

kirankt · 2021-12-01T17:56:34Z

pkg/asset/machines/worker.go

@@ -555,7 +552,6 @@ func (w *Worker) MachineSets() ([]machineapi.MachineSet, error) {
 		awsprovider.SchemeGroupVersion,
 		azureprovider.SchemeGroupVersion,
 		baremetalprovider.SchemeGroupVersion,
-		gcpprovider.SchemeGroupVersion,


staebler

Please separate the vendor changes in their own commit.

staebler · 2021-12-01T18:31:47Z

pkg/tfvars/gcp/gcp.go

@@ -4,8 +4,7 @@ import (
 	"encoding/json"
 	"fmt"

-	gcpprovider "github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1"
-
+	machineapi "github.com/openshift/api/machine/v1beta1"


Leave the machineapi import in a section separate from the installer import.

patrickdillon · 2021-12-01T19:18:47Z

/test e2e-gcp

Ok, fixed it up based on the recommendations. Let's see if we get further in the tests now.

patrickdillon · 2021-12-02T00:22:59Z

install suceeds, 2 failed tests that look unrelated

/test e2e-gcp

kirankt

It would be nice to see 2 separate commits. One for the code changes and the other for vendoring changes. Other than that it looks like the gcp test is passing, so lgtm.

kirankt · 2021-12-02T16:18:20Z

/test e2e-metal

jhixson74 · 2021-12-02T21:44:56Z

/uncc

patrickdillon · 2021-12-14T21:25:08Z

Oops, I lost track of this but have now updated/rebased.

staebler · 2021-12-14T21:36:40Z

/test e2e-gcp

kirankt

GCP test passed.
/lgtm

staebler · 2021-12-15T13:16:55Z

/approve

openshift-ci · 2021-12-15T13:17:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: staebler

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~~ [staebler]

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

openshift-bot · 2021-12-15T14:38:42Z

/retest-required

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

openshift-bot · 2021-12-15T23:55:11Z

/retest-required

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

openshift-bot · 2021-12-16T03:07:11Z

/retest-required

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

openshift-bot · 2021-12-16T06:10:41Z

/retest-required

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

openshift-bot · 2021-12-16T06:49:02Z

/retest-required

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

openshift-bot · 2021-12-16T08:07:01Z

/retest-required

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

openshift-bot · 2021-12-16T09:24:07Z

/retest-required

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

openshift-bot · 2021-12-16T09:48:08Z

/retest-required

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

openshift-bot · 2021-12-16T11:00:08Z

/retest-required

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

openshift-ci · 2021-12-16T11:16:19Z

@patrickdillon: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/verify-vendor	98e2d2a30d1116bc0c11ea2805febe87e8d41c24	link	true	`/test verify-vendor`

Full PR test history. Your PR dashboard.

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-bot · 2021-12-16T12:24:22Z

/retest-required

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

openshift-bot · 2021-12-16T12:50:22Z

/retest-required

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

Now that the GCP machine provider spec has moved to the openshift/api we can remove the GCP cluster api provider.

staebler · 2021-12-16T13:13:50Z

98e2d2a30...3e4b438b1

Rebased and ran go mod tidy to fix up the go.sum file.

/lgtm

staebler · 2021-12-16T13:39:26Z

3e4b438b1...830d0d73f

Actually include the go.sum changes this time.

/lgtm

openshift-bot · 2021-12-16T15:07:31Z

/retest-required

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

openshift-bot · 2021-12-16T15:19:21Z

/retest-required

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

openshift-bot · 2021-12-16T16:55:28Z

/retest-required

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

openshift-bot · 2021-12-16T17:07:20Z

/retest-required

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

openshift-bot · 2021-12-16T18:08:28Z

/retest-required

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

openshift-bot · 2021-12-16T18:33:19Z

/retest-required

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

openshift-bot · 2021-12-16T20:09:20Z

/retest-required

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

openshift-ci bot requested review from kirankt and staebler December 1, 2021 17:14

openshift-ci bot requested review from fabianofranz and jhixson74 December 1, 2021 17:16

patrickdillon force-pushed the remove-gcp-cluster-api-provider branch from 346d380 to 4737c4a Compare December 1, 2021 17:17

kirankt reviewed Dec 1, 2021

View reviewed changes

staebler reviewed Dec 1, 2021

View reviewed changes

patrickdillon force-pushed the remove-gcp-cluster-api-provider branch 2 times, most recently from 452963a to 173546b Compare December 1, 2021 19:17

kirankt reviewed Dec 2, 2021

View reviewed changes

openshift-ci bot removed the request for review from jhixson74 December 2, 2021 21:44

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2021

patrickdillon force-pushed the remove-gcp-cluster-api-provider branch from 173546b to 98e2d2a Compare December 14, 2021 21:24

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 14, 2021

kirankt reviewed Dec 15, 2021

View reviewed changes

openshift-ci bot assigned kirankt Dec 15, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 15, 2021

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 15, 2021

Remove GCP Cluster API Provider

ea14f9d

Now that the GCP machine provider spec has moved to the openshift/api we can remove the GCP cluster api provider.

staebler force-pushed the remove-gcp-cluster-api-provider branch from 98e2d2a to 3e4b438 Compare December 16, 2021 13:12

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2021

openshift-ci bot assigned staebler Dec 16, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2021

vendor: remove unused cluster-api-provider-gcp bits

830d0d7

staebler force-pushed the remove-gcp-cluster-api-provider branch from 3e4b438 to 830d0d7 Compare December 16, 2021 13:38

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2021

openshift-merge-robot merged commit 8f24215 into openshift:master Dec 16, 2021

	// Add vsphere types to scheme
	scheme.AddKnownTypes(machineapi.SchemeGroupVersion,
	&machineapi.VSphereMachineProviderSpec{},
	)

Remove GCP cluster api provider #5437

Remove GCP cluster api provider #5437

Conversation

patrickdillon commented Dec 1, 2021

patrickdillon commented Dec 1, 2021

patrickdillon commented Dec 1, 2021

patrickdillon commented Dec 1, 2021

kirankt left a comment

Choose a reason for hiding this comment

kirankt Dec 1, 2021

Choose a reason for hiding this comment

kirankt Dec 1, 2021

Choose a reason for hiding this comment

staebler left a comment

Choose a reason for hiding this comment

staebler Dec 1, 2021

Choose a reason for hiding this comment

patrickdillon commented Dec 1, 2021

patrickdillon commented Dec 2, 2021

kirankt left a comment

Choose a reason for hiding this comment

kirankt commented Dec 2, 2021

jhixson74 commented Dec 2, 2021

patrickdillon commented Dec 14, 2021

staebler commented Dec 14, 2021

kirankt left a comment

Choose a reason for hiding this comment

staebler commented Dec 15, 2021

openshift-ci bot commented Dec 15, 2021

openshift-bot commented Dec 15, 2021

openshift-bot commented Dec 15, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-ci bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

staebler commented Dec 16, 2021

staebler commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021

openshift-bot commented Dec 16, 2021