OCPBUGS-35343: make shutdown-delay-duration configurable #1685

tkashem · 2024-05-21T14:28:09Z

No description provided.

openshift-ci-robot · 2024-05-21T14:28:15Z

@tkashem: This pull request references Jira Issue OCPBUGS-30860, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.17.0) matches configured target version for branch (4.17.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @geliu2016

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

tkashem · 2024-05-21T14:28:34Z

/cc @p0lyn0mial @vrutkovs

openshift-ci · 2024-05-21T14:40:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tkashem

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

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

tkashem · 2024-05-21T16:02:23Z

/retest-required

openshift-ci · 2024-05-21T17:48:30Z

@tkashem: The following tests 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/unit	`681dd36`	link	true	`/test unit`
ci/prow/e2e-metal-single-node-live-iso	`681dd36`	link	false	`/test e2e-metal-single-node-live-iso`
ci/prow/k8s-e2e-gcp	`681dd36`	link	true	`/test k8s-e2e-gcp`
ci/prow/e2e-aws-operator-disruptive-single-node	`681dd36`	link	false	`/test e2e-aws-operator-disruptive-single-node`
ci/prow/e2e-aws-ovn-single-node	`681dd36`	link	false	`/test e2e-aws-ovn-single-node`
ci/prow/e2e-gcp-operator-single-node	`681dd36`	link	false	`/test e2e-gcp-operator-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-sigs/prow repository. I understand the commands that are listed here.

p0lyn0mial · 2024-05-22T06:53:24Z

pkg/cmd/render/render.go

@@ -97,6 +106,7 @@ func (r *renderOpts) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&r.clusterConfigFile, "cluster-config-file", r.clusterConfigFile, "Openshift Cluster API Config file.")
 	fs.StringVar(&r.clusterAuthFile, "cluster-auth-file", r.clusterAuthFile, "Openshift Cluster Authentication API Config file.")
 	fs.StringVar(&r.infraConfigFile, "infra-config-file", "", "File containing infrastructure.config.openshift.io manifest.")
+	fs.DurationVar(&r.shutdownDelayDuration, "shutdown-delay-duration", r.shutdownDelayDuration, "shutdown-delay-duration argument for the bootstrap kube-apiserver.")


why not to add it directly to the bootstrap-api-server's manifest ? - https://github.com/openshift/cluster-kube-apiserver-operator/blob/master/bindata/bootkube/bootstrap-manifests/kube-apiserver-pod.yaml#L46

bootkube already has an override -

cluster-kube-apiserver-operator/bindata/bootkube/config/bootstrap-config-overrides.yaml

Lines 79 to 81 in 04eba64

{{- if .ShutdownDelayDuration}}

shutdown-delay-duration:

- {{ .ShutdownDelayDuration }}

oh, cool!
should we also set shutdown-send-retry-after ?

p0lyn0mial · 2024-05-23T06:47:24Z

pkg/cmd/render/render.go

@@ -192,7 +206,7 @@ func (r *renderOpts) Run() error {
 		BindAddress:                   "0.0.0.0:6443",
 		BindNetwork:                   "tcp4",
 		TerminationGracePeriodSeconds: 135, // bit more than 70s (minimal termination period) + 60s (apiserver graceful termination)
-		ShutdownDelayDuration:         "",  // do not override
+		ShutdownDelayDuration:         r.shutdownDelayDuration.String(),


BTW: Do we want to expose it as a command line arg ? If yes then we have to coordinate updating it with terminationGracePeriodSeconds. Otherwise kubelet might terminate kas process before graceful termination completes.

p0lyn0mial · 2024-05-23T06:56:56Z

pkg/cmd/render/render.go

+		// by default, we are giving the load balancer 20s to remove the
+		// bootstrap kube-apiserver from its pool after TERM signal is
+		// sent to the kube-apiserver on the bootstrap node.
+		shutdownDelayDuration: 20 * time.Second,


What is the default value for shutdownDelayDuration ?
It looks like it is 0, does it mean that the bootstrap api server didn't give any time to LBs to react ?

iiuc its ) for SNO and 129 for AWS:

cluster-kube-apiserver-operator/pkg/operator/configobservation/apiserver/observe_termination_duration.go

Lines 37 to 46 in 37df1b1

case infra.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode:

// reduce the shutdown delay to 0 to reach the maximum downtime for SNO

observedShutdownDelayDuration = "0s"

case infra.Spec.PlatformSpec.Type == configv1.AWSPlatformType:

// AWS has a known issue: https://bugzilla.redhat.com/show_bug.cgi?id=1943804

// We need to extend the shutdown-delay-duration so that an NLB has a chance to notice and remove unhealthy instance.

// Once the mentioned issue is resolved this code must be removed and default values applied

//

// Note this is the official number we got from AWS

observedShutdownDelayDuration = "129s"

vrutkovs · 2024-06-12T12:39:10Z

/retitle OCPBUGS-35343: make shutdown-delay-duration configurable

openshift-ci-robot · 2024-06-12T12:39:35Z

@tkashem: This pull request references Jira Issue OCPBUGS-35343, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.17.0) matches configured target version for branch (4.17.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @wangke19

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

make shutdown-delay-duration configurable

681dd36

openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels May 21, 2024

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label May 21, 2024

openshift-ci bot requested review from geliu2016, p0lyn0mial, vrutkovs and deads2k May 21, 2024 14:28

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 21, 2024

p0lyn0mial reviewed May 22, 2024

View reviewed changes

p0lyn0mial reviewed May 23, 2024

View reviewed changes

openshift-ci-robot mentioned this pull request May 29, 2024

OCPBUGS-30860: Use infrastructure API to detect cluster topology openshift/installer#8429

Merged

openshift-ci bot changed the title ~~OCPBUGS-30860: make shutdown-delay-duration configurable~~ OCPBUGS-35343: make shutdown-delay-duration configurable Jun 12, 2024

openshift-ci-robot removed the jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. label Jun 12, 2024

openshift-ci bot requested a review from wangke19 June 12, 2024 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-35343: make shutdown-delay-duration configurable #1685

OCPBUGS-35343: make shutdown-delay-duration configurable #1685

tkashem commented May 21, 2024

openshift-ci-robot commented May 21, 2024

tkashem commented May 21, 2024

openshift-ci bot commented May 21, 2024

tkashem commented May 21, 2024

openshift-ci bot commented May 21, 2024

p0lyn0mial May 22, 2024

tkashem May 22, 2024

p0lyn0mial May 23, 2024

p0lyn0mial May 23, 2024

p0lyn0mial May 23, 2024

vrutkovs Sep 3, 2024

vrutkovs commented Jun 12, 2024

openshift-ci-robot commented Jun 12, 2024

	{{- if .ShutdownDelayDuration}}
	shutdown-delay-duration:
	- {{ .ShutdownDelayDuration }}

	case infra.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode:
	// reduce the shutdown delay to 0 to reach the maximum downtime for SNO
	observedShutdownDelayDuration = "0s"
	case infra.Spec.PlatformSpec.Type == configv1.AWSPlatformType:
	// AWS has a known issue: https://bugzilla.redhat.com/show_bug.cgi?id=1943804
	// We need to extend the shutdown-delay-duration so that an NLB has a chance to notice and remove unhealthy instance.
	// Once the mentioned issue is resolved this code must be removed and default values applied
	//
	// Note this is the official number we got from AWS
	observedShutdownDelayDuration = "129s"

OCPBUGS-35343: make shutdown-delay-duration configurable #1685

Are you sure you want to change the base?

OCPBUGS-35343: make shutdown-delay-duration configurable #1685

Conversation

tkashem commented May 21, 2024

openshift-ci-robot commented May 21, 2024

tkashem commented May 21, 2024

openshift-ci bot commented May 21, 2024

tkashem commented May 21, 2024

openshift-ci bot commented May 21, 2024

p0lyn0mial May 22, 2024

Choose a reason for hiding this comment

tkashem May 22, 2024

Choose a reason for hiding this comment

p0lyn0mial May 23, 2024

Choose a reason for hiding this comment

p0lyn0mial May 23, 2024

Choose a reason for hiding this comment

p0lyn0mial May 23, 2024

Choose a reason for hiding this comment

vrutkovs Sep 3, 2024

Choose a reason for hiding this comment

vrutkovs commented Jun 12, 2024

openshift-ci-robot commented Jun 12, 2024