Skip to content

Comments

Move to libraries to openshift/api#20

Merged
openshift-merge-robot merged 2 commits intoopenshift:mainfrom
kwoodson:move_to_openshiftapi
Dec 13, 2021
Merged

Move to libraries to openshift/api#20
openshift-merge-robot merged 2 commits intoopenshift:mainfrom
kwoodson:move_to_openshiftapi

Conversation

@kwoodson
Copy link
Contributor

This PR allows us to move our libraries to openshift/api from cluster-api-provider-alibaba. This depends on github.com/openshift/api/pull/1045.

Summary

  • Remove API and generated files
  • Plumb partially existing fields
  • Update vendor

@kwoodson kwoodson requested review from JoelSpeed and elmiko November 29, 2021 18:03
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 29, 2021
@openshift-ci openshift-ci bot requested review from Fedosin and lobziik November 29, 2021 18:05
@kwoodson
Copy link
Contributor Author

Tested this change. Everything looked healthy.

NAME                                 STATUS   ROLES    AGE     VERSION
test-kdlxs-master-0                  Ready    master   19m     v1.22.1+bac83a5
test-kdlxs-master-1                  Ready    master   20m     v1.22.1+bac83a5
test-kdlxs-master-2                  Ready    master   25s     v1.22.1+bac83a5
test-kdlxs-worker-us-east-1a-dqr7r   Ready    worker   7m38s   v1.22.1+bac83a5
test-kdlxs-worker-us-east-1b-rdkhq   Ready    worker   8m32s   v1.22.1+bac83a5
test-kdlxs-worker-us-east-1b-ttqsr   Ready    worker   8m10s   v1.22.1+bac83a5
NAME                                 PHASE     TYPE            REGION	   ZONE         AGE
test-kdlxs-master-0                  Running   ecs.g6.xlarge   us-east-1   us-east-1b   21m
test-kdlxs-master-1                  Running   ecs.g6.xlarge   us-east-1   us-east-1a   21m
test-kdlxs-master-2                  Running   ecs.g6.xlarge   us-east-1   us-east-1b   21m
test-kdlxs-worker-us-east-1a-dqr7r   Running   ecs.g6.large    us-east-1   us-east-1a   14m
test-kdlxs-worker-us-east-1b-rdkhq   Running   ecs.g6.large    us-east-1   us-east-1b   14m

@kwoodson kwoodson changed the title [WIP] Move to libraries to openshift/api Move to libraries to openshift/api Nov 30, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 30, 2021
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Changes here look good, just need to wait for 1045 to merge on the API side

@kwoodson kwoodson force-pushed the move_to_openshiftapi branch from df2b2da to 767bb8f Compare November 30, 2021 17:01
@kwoodson
Copy link
Contributor Author

Cleaned up openshift/api/pull/1045 and tested:

NAME                                 STATUS   ROLES    AGE     VERSION
test-q92hx-master-0                  Ready    master   18m     v1.22.1+bac83a5
test-q92hx-master-1                  Ready    master   17m     v1.22.1+bac83a5
test-q92hx-master-2                  Ready    master   19m     v1.22.1+bac83a5
test-q92hx-worker-us-east-1a-wwznl   Ready    worker   5m3s    v1.22.1+bac83a5
test-q92hx-worker-us-east-1b-lmtvq   Ready    worker   3m39s   v1.22.1+bac83a5
test-q92hx-worker-us-east-1b-z5kb7   Ready    worker   5m54s   v1.22.1+bac83a5
NAME                                 PHASE     TYPE            REGION      ZONE         AGE
test-q92hx-master-0                  Running   ecs.g6.xlarge   us-east-1   us-east-1b   20m
test-q92hx-master-1                  Running   ecs.g6.xlarge   us-east-1   us-east-1a   20m
test-q92hx-master-2                  Running   ecs.g6.xlarge   us-east-1   us-east-1b   20m
test-q92hx-worker-us-east-1a-wwznl   Running   ecs.g6.large    us-east-1   us-east-1a   12m
test-q92hx-worker-us-east-1b-lmtvq   Running   ecs.g6.large    us-east-1   us-east-1b   12m
test-q92hx-worker-us-east-1b-z5kb7   Running   ecs.g6.large    us-east-1   us-east-1b   12m

@kwoodson kwoodson force-pushed the move_to_openshiftapi branch 4 times, most recently from 4d262ef to 0c27273 Compare December 6, 2021 20:43
Copy link
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

the machine object is still at v1beta1, otherwise looks good

README.md Outdated

```yaml
apiVersion: machine.openshift.io/v1beta1
apiVersion: machine.openshift.io/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

this would stay at v1beta1

providerSpec:
value:
apiVersion: alibabacloudproviderconfig.openshift.io/v1alpha1
apiVersion: alibabacloudproviderconfig.openshift.io/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm pretty sure these should be machine.openshift.io/v1 too

Copy link
Contributor

Choose a reason for hiding this comment

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

@kwoodson
Copy link
Contributor Author

kwoodson commented Dec 9, 2021

Working out a few issues from pulling down latest openshift/api. Going to move to go1.17 in #21 and then rebase this PR on top of that one.

@kwoodson kwoodson force-pushed the move_to_openshiftapi branch from f683467 to c88700e Compare December 9, 2021 21:16
@kwoodson kwoodson force-pushed the move_to_openshiftapi branch from c88700e to 492d5f4 Compare December 9, 2021 21:53
@kwoodson
Copy link
Contributor Author

/test vet

@JoelSpeed
Copy link
Contributor

CI is running as 1.16 for this repo still, will need to update that for vet to pass

@kwoodson
Copy link
Contributor Author

/test vet

@kwoodson kwoodson mentioned this pull request Dec 10, 2021
@kwoodson
Copy link
Contributor Author

Latest version installs:

NAME                                 STATUS   ROLES    AGE     VERSION
test-4627z-master-0                  Ready    master   13m     v1.22.1+6859754
test-4627z-master-1                  Ready    master   14m     v1.22.1+6859754
test-4627z-master-2                  Ready	master   14m     v1.22.1+6859754
test-4627z-worker-us-east-1a-74fvh   Ready    worker   2m10s   v1.22.1+6859754
test-4627z-worker-us-east-1b-rt76n   Ready    worker   2m2s    v1.22.1+6859754
test-4627z-worker-us-east-1b-wwxs8   Ready    worker   2m8s    v1.22.1+6859754
NAME                                 PHASE     TYPE            REGION      ZONE         AGE
test-4627z-master-0                  Running   ecs.g6.xlarge   us-east-1   us-east-1b   16m
test-4627z-master-1                  Running   ecs.g6.xlarge   us-east-1   us-east-1a   16m
test-4627z-master-2                  Running   ecs.g6.xlarge   us-east-1   us-east-1b   16m
test-4627z-worker-us-east-1a-74fvh   Running   ecs.g6.large    us-east-1   us-east-1a   10m
test-4627z-worker-us-east-1b-rt76n   Running   ecs.g6.large    us-east-1   us-east-1b   10m
test-4627z-worker-us-east-1b-wwxs8   Running   ecs.g6.large    us-east-1   us-east-1b   10m

@kwoodson
Copy link
Contributor Author

@elmiko @JoelSpeed I think this is ready to go. PTAL

@JoelSpeed
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 13, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, kwoodson

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2021
@kwoodson
Copy link
Contributor Author

@elmiko @JoelSpeed Is there any reason we run aws tests on this repository?

@kwoodson
Copy link
Contributor Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 13, 2021

@kwoodson: all tests passed!

Full PR test history. Your PR dashboard.

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.

@elmiko
Copy link
Contributor

elmiko commented Dec 13, 2021

Is there any reason we run aws tests on this repository?

the aws test is like a "sanity check" for us, it allows us to ensure that we aren't breaking the primary payload with our changes. (that said, i know we see flakes there sometimes, but we need to have at least 1 e2e on a known GA platform)

Copy link
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

thanks Kenny!
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2021
@openshift-merge-robot openshift-merge-robot merged commit a748e09 into openshift:main Dec 13, 2021
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants