Skip to content

Conversation

hexfusion
Copy link
Contributor

@hexfusion hexfusion commented Feb 24, 2021

Currently, the bootstrap in place used with SNO adds an annotation alpha.installer.openshift.io/etcd-bootstrap which is intended to be used by the cluster to communicate with the bootstrap etcd instance. But bootstrap in place means that the bootstrap node will never be visible. The result is the cluster never pivots from the bootstrap node endpoint which is not expected. This change fixes that problem by adding the BootstrapInPlaceStrategy which omits that annotation.

  • refactor scaling strategies and add BootstrapInPlaceStrategy
  • add unit tests

@openshift-ci-robot 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 Feb 24, 2021
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2021
@marun
Copy link
Contributor

marun commented Feb 25, 2021

Why is it desirable to switch to defining the manifests in code? Is there a limitation of template substitution that suggests this change?

@hexfusion hexfusion force-pushed the sno-skip-bootstrap-endpoint branch 2 times, most recently from ec84b3b to c799473 Compare February 25, 2021 17:48
@hexfusion
Copy link
Contributor Author

Why is it desirable to switch to defining the manifests in code? Is there a limitation of template substitution that suggests this change?

In the case of conditional changes against the resource using the raw object seems more natural vs rendering template variables with conditional logic. We could use a base object and append then rewrite to disk but it feels strange.

@hexfusion hexfusion changed the title [wip] render: refactor scaling strategies render: refactor scaling strategies Feb 25, 2021
@openshift-ci-robot 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 Feb 25, 2021
@hexfusion hexfusion force-pushed the sno-skip-bootstrap-endpoint branch 3 times, most recently from 6f728c5 to aa330ca Compare February 25, 2021 20:07
@hexfusion hexfusion changed the title render: refactor scaling strategies Bug 1931658: render: refactor scaling strategies Feb 25, 2021
@openshift-ci-robot
Copy link

@hexfusion: This pull request references Bugzilla bug 1931658, 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.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1931658: render: refactor scaling strategies

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 bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Feb 25, 2021
@openshift-ci-robot
Copy link

@hexfusion: This pull request references Bugzilla bug 1931658, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1931658: render: refactor scaling strategies

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.

@hexfusion
Copy link
Contributor Author

/test e2e-metal-single-node-live-iso
/test e2e-aws-single-node

@hexfusion
Copy link
Contributor Author

/hold for sno tests

@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 Feb 25, 2021
@hexfusion
Copy link
Contributor Author

Multi-AZ Clusters should spread the pods of a replication controller across zones

agnostic fail looks like https://bugzilla.redhat.com/show_bug.cgi?id=1929389

@hexfusion
Copy link
Contributor Author

/test e2e-agnostic

@hexfusion
Copy link
Contributor Author

Copy link
Contributor

@marun marun left a comment

Choose a reason for hiding this comment

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

/lgtm

Squash for merge?

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 26, 2021
@romfreiman
Copy link

e2e-aws-single-node

This test is not setting the bootstrapInPlace[1] in the install-config.

[1] https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-etcd-operator/547/pull-ci-openshift-cluster-etcd-operator-master-e2e-aws-single-node/1365064469167738880/artifacts/e2e-aws-single-node/gather-extra/artifacts/configmaps.json

cc @romfreiman @tsorya

Right. It uses bootstrap vm on aws.
Bootstrap in place is used only with the e2e-metal-live-iso test

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

/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 Mar 16, 2021
@hexfusion hexfusion force-pushed the sno-skip-bootstrap-endpoint branch from df6f35c to 7ead1a3 Compare March 16, 2021 03:22
@hexfusion
Copy link
Contributor Author

infra
/retest

@hexfusion
Copy link
Contributor Author

added fixup

@hexfusion hexfusion force-pushed the sno-skip-bootstrap-endpoint branch 3 times, most recently from 18ce572 to 37903cd Compare March 17, 2021 13:38
@hexfusion
Copy link
Contributor Author

rebased ready for review

@hexfusion hexfusion force-pushed the sno-skip-bootstrap-endpoint branch from 37903cd to 26c8f02 Compare March 17, 2021 13:53
@hexfusion
Copy link
Contributor Author

/retest

@romfreiman
Copy link

/test e2e-metal-single-node-live-iso

Copy link
Contributor

@marun marun left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021
@hexfusion hexfusion force-pushed the sno-skip-bootstrap-endpoint branch from 26c8f02 to 20c80c5 Compare March 18, 2021 01:27
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2021
@hexfusion
Copy link
Contributor Author

/retest

@marun
Copy link
Contributor

marun commented Mar 18, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2021
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hexfusion, marun

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

@hexfusion
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 21, 2021

@hexfusion: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-single-node efda532 link /test e2e-aws-single-node

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
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 05079e3 into openshift:master Mar 21, 2021
@openshift-ci-robot
Copy link

@hexfusion: All pull requests linked via external trackers have merged:

Bugzilla bug 1931658 has been moved to the MODIFIED state.

In response to this:

Bug 1931658: render: refactor scaling strategies

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.

@hexfusion hexfusion changed the title Bug 1931658: render: refactor scaling strategies ETCD-179: Bug 1931658: render: refactor scaling strategies Mar 22, 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. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants