Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch to server-side Apply on cluster create #168

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Switch to server-side Apply on cluster create #168

merged 1 commit into from
Apr 21, 2021

Conversation

csrwng
Copy link
Contributor

@csrwng csrwng commented Apr 20, 2021

Ensures that the content of existing secrets with the same name are overwritten with whatever is getting created.

@csrwng csrwng requested a review from ironcladlou April 20, 2021 23:14
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csrwng

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:

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 Apr 20, 2021
enxebre
enxebre reviewed Apr 21, 2021
View reviewed changes
cmd/cluster/create.go
@@ -253,11 +253,11 @@ func CreateCluster(ctx context.Context, opts Options) error {
default:
for _, object := range exampleObjects {
key := crclient.ObjectKeyFromObject(object)
_, err = controllerutil.CreateOrUpdate(ctx, client, object, NoopReconcile)
Copy link
Member

Choose a reason for hiding this comment

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

nit: we can inline this if now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, fixed

@enxebre
Copy link
Member

enxebre commented Apr 21, 2021

We'll want to use kubernetes-sigs/controller-runtime#1474 for all our patching operations once it makes it through.

@enxebre
Copy link
Member

enxebre commented Apr 21, 2021

/test aws-e2e

@openshift-ci-robot
Copy link

@enxebre: The specified target(s) for /test were not found.
The following commands are available to trigger jobs:

  • /test e2e-aws
  • /test images
  • /test unit
  • /test verify

Use /test all to run the following jobs:

  • pull-ci-openshift-hypershift-main-images
  • pull-ci-openshift-hypershift-main-unit
  • pull-ci-openshift-hypershift-main-verify

In response to this:

/test aws-e2e

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.

@enxebre
Copy link
Member

enxebre commented Apr 21, 2021

/test e2e-aws

@csrwng
Copy link
Contributor Author

csrwng commented Apr 21, 2021

/test e2e-aws

@sjenning
Copy link
Contributor

/lgtm
/hold
for e2e completion

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 21, 2021
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 21, 2021
@csrwng
Copy link
Contributor Author

csrwng commented Apr 21, 2021

/test e2e-aws

@csrwng
Copy link
Contributor Author

csrwng commented Apr 21, 2021

Previous test run failed with:

[36mINFO�[0m[2021-04-21T14:39:01Z] time="2021-04-21T14:38:33Z" level=info msg="found stack event" LogicalResourceId=example-ptj2w-65qnv ResourceStatus=ROLLBACK_IN_PROGRESS ResourceStatusReason="The following resource(s) failed to create: [SubdomainPublicZone, OIDCImageRegistryRole, OIDCBucket, WorkerInstanceProfile, OIDCIngressRole, OIDCCSIDriverRole, NodePoolManagementUser, KubeCloudControllerUser]. Rollback requested by user." ResourceType="AWS::CloudFormation::Stack"

@csrwng
Copy link
Contributor Author

csrwng commented Apr 21, 2021

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 21, 2021
@openshift-merge-robot openshift-merge-robot merged commit 69bfbd5 into openshift:main Apr 21, 2021
@csrwng csrwng deleted the apply_on_create branch June 14, 2022 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enxebre enxebre enxebre left review comments

@ironcladlou ironcladlou Awaiting requested review from ironcladlou

@sjenning sjenning Awaiting requested review from sjenning

Assignees

@sjenning sjenning

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@csrwng @openshift-ci-robot @enxebre @sjenning @openshift-merge-robot