OCPBUGS-20024: Revert "Revert "Set DNS DaemonSet's maxSurge value to 10%"" #384

Miciah · 2023-10-03T00:14:31Z

The defect in the daemon controller, OCPBUGS-19452 that prevented us from using maxSurge has been fixed by openshift/kubernetes#1716.

This reverts #379.

The defect in the daemon controller, <https://issues.redhat.com/browse/OCPBUGS-15531>, that prevented us from using maxSurge has been fixed. This commit is related to OCPBUGS-20024. https://issues.redhat.com/browse/OCPBUGS-20024. This reverts commit 1f7ac36.

openshift-ci-robot · 2023-10-03T00:14:38Z

@Miciah: This pull request references Jira Issue OCPBUGS-20024, 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.15.0) matches configured target version for branch (4.15.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @melvinjoseph86

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

In response to this:

The defect in the daemon controller, OCPBUGS-19452 that prevented us from using maxSurge has been fixed by openshift/kubernetes#1716.

This reverts #379.

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 · 2023-10-03T00:14:40Z

@Miciah: This pull request references Jira Issue OCPBUGS-20024, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @melvinjoseph86

In response to this:

The defect in the daemon controller, OCPBUGS-19452 that prevented us from using maxSurge has been fixed by openshift/kubernetes#1716.

This reverts #379.

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 · 2023-10-03T02:48:00Z

@Miciah: all tests passed!

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-merge-robot · 2023-10-03T02:48:00Z

@Miciah: all tests passed!

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.

candita · 2023-10-04T15:39:56Z

/assign @alebedev87

alebedev87 · 2023-10-05T15:02:23Z

/lgtm

Miciah · 2023-10-05T15:06:23Z

For the record, I tested the changes in this PR manually in order to verify that OCPBUGS-19452 really is fixed and that OCPBUGS-13209 cannot be reproduced with the changes in this PR:

% oc -n openshift-dns get ds dns-default -o yaml | yq .spec.updateStrategy
{
  "rollingUpdate": {
    "maxSurge": "10%",
    "maxUnavailable": 0
  },
  "type": "RollingUpdate"
}
% oc  -n openshift-dns get ds dns-default -o yaml | yq .spec.template.spec.tolerations
[
  {
    "key": "node-role.kubernetes.io/master",
    "operator": "Exists"
  }
]
% oc -n openshift-dns get pods  -o wide | grep dns-default
dns-default-4gsp8     2/2     Running   0          125m   10.131.0.7     ip-10-0-82-49.us-west-2.compute.internal     <none>           <none>
dns-default-6bvkd     2/2     Running   0          80m    10.129.2.6     ip-10-0-30-152.us-west-2.compute.internal    <none>           <none>
dns-default-fcfgj     2/2     Running   0          131m   10.128.0.16    ip-10-0-103-228.us-west-2.compute.internal   <none>           <none>
dns-default-fz6xn     2/2     Running   0          125m   10.128.2.6     ip-10-0-92-150.us-west-2.compute.internal    <none>           <none>
dns-default-hwbhr     2/2     Running   0          131m   10.129.0.42    ip-10-0-121-112.us-west-2.compute.internal   <none>           <none>
dns-default-zd898     2/2     Running   0          131m   10.130.0.14    ip-10-0-46-71.us-west-2.compute.internal     <none>           <none>
% oc -n openshift-dns get ds dns-default -o yaml | yq .spec.template.spec.tolerations
[
  {
    "key": "node-role.kubernetes.io/master",
    "operator": "Exists"
  }
]
% oc patch dnses.operator.openshift.io/default --type=merge --patch='
spec:
  nodePlacement:
    tolerations:
    - key: test-taint
      operator: Exists
'
dns.operator.openshift.io/default patched
% oc -n openshift-dns get ds dns-default -o yaml | yq .spec.template.spec.tolerations
[
  {
    "key": "test-taint",
    "operator": "Exists"
  }
]
% oc -n openshift-dns get pods  -o wide | grep dns-default
dns-default-4gsp8     2/2     Running       0          127m   10.131.0.7     ip-10-0-82-49.us-west-2.compute.internal     <none>           <none>
dns-default-6bvkd     2/2     Running       0          82m    10.129.2.6     ip-10-0-30-152.us-west-2.compute.internal    <none>           <none>
dns-default-6khq6     2/2     Running       0          15s    10.131.0.21    ip-10-0-82-49.us-west-2.compute.internal     <none>           <none>
dns-default-fcfgj     1/2     Terminating   0          133m   10.128.0.16    ip-10-0-103-228.us-west-2.compute.internal   <none>           <none>
dns-default-fz6xn     2/2     Running       0          127m   10.128.2.6     ip-10-0-92-150.us-west-2.compute.internal    <none>           <none>
dns-default-hwbhr     1/2     Terminating   0          133m   10.129.0.42    ip-10-0-121-112.us-west-2.compute.internal   <none>           <none>
dns-default-zd898     1/2     Terminating   0          133m   10.130.0.14    ip-10-0-46-71.us-west-2.compute.internal     <none>           <none>
% oc -n openshift-dns get pods  -o wide | grep dns-default
dns-default-6khq6     2/2     Running   0          3m16s   10.131.0.21    ip-10-0-82-49.us-west-2.compute.internal     <none>           <none>
dns-default-lm7dq     2/2     Running   0          2m34s   10.128.2.34    ip-10-0-92-150.us-west-2.compute.internal    <none>           <none>
dns-default-qgspt     2/2     Running   0          2m55s   10.129.2.8     ip-10-0-30-152.us-west-2.compute.internal    <none>           <none>
% 
% oc get nodes -o wide
NAME                                         STATUS   ROLES                  AGE    VERSION           INTERNAL-IP    EXTERNAL-IP   OS-IMAGE                                                       KERNEL-VERSION                 CONTAINER-RUNTIME
ip-10-0-103-228.us-west-2.compute.internal   Ready    control-plane,master   142m   v1.28.2+6ab54b8   10.0.103.228   <none>        Red Hat Enterprise Linux CoreOS 415.92.202310040212-0 (Plow)   5.14.0-284.34.1.el9_2.x86_64   cri-o://1.28.1-3.rhaos4.15.gitb36169e.el9
ip-10-0-121-112.us-west-2.compute.internal   Ready    control-plane,master   141m   v1.28.2+6ab54b8   10.0.121.112   <none>        Red Hat Enterprise Linux CoreOS 415.92.202310040212-0 (Plow)   5.14.0-284.34.1.el9_2.x86_64   cri-o://1.28.1-3.rhaos4.15.gitb36169e.el9
ip-10-0-30-152.us-west-2.compute.internal    Ready    worker                 89m    v1.28.2+6ab54b8   10.0.30.152    <none>        Red Hat Enterprise Linux CoreOS 415.92.202310040212-0 (Plow)   5.14.0-284.34.1.el9_2.x86_64   cri-o://1.28.1-3.rhaos4.15.gitb36169e.el9
ip-10-0-46-71.us-west-2.compute.internal     Ready    control-plane,master   141m   v1.28.2+6ab54b8   10.0.46.71     <none>        Red Hat Enterprise Linux CoreOS 415.92.202310040212-0 (Plow)   5.14.0-284.34.1.el9_2.x86_64   cri-o://1.28.1-3.rhaos4.15.gitb36169e.el9
ip-10-0-82-49.us-west-2.compute.internal     Ready    worker                 134m   v1.28.2+6ab54b8   10.0.82.49     <none>        Red Hat Enterprise Linux CoreOS 415.92.202310040212-0 (Plow)   5.14.0-284.34.1.el9_2.x86_64   cri-o://1.28.1-3.rhaos4.15.gitb36169e.el9
ip-10-0-92-150.us-west-2.compute.internal    Ready    worker                 134m   v1.28.2+6ab54b8   10.0.92.150    <none>        Red Hat Enterprise Linux CoreOS 415.92.202310040212-0 (Plow)   5.14.0-284.34.1.el9_2.x86_64   cri-o://1.28.1-3.rhaos4.15.gitb36169e.el9

candita · 2023-10-05T22:24:54Z

pkg/operator/controller/controller_dns_daemonset_test.go

+		{
+			description: "if the update strategy's max surge parameter changes",
+			mutate: func(daemonset *appsv1.DaemonSet) {
+				daemonset.Spec.UpdateStrategy.RollingUpdate.MaxSurge = pointerTo(intstr.FromString("10%"))


This is not a change, is it?

I guess it is. I expected that you would have used the manifest values as the base, but upon further checking, you didn't.

candita · 2023-10-05T22:25:25Z

pkg/operator/controller/controller_dns_daemonset_test.go

+		{
+			description: "if spec.minReadySeconds changes",
+			mutate: func(daemonset *appsv1.DaemonSet) {
+				daemonset.Spec.MinReadySeconds = 9


This is also not a change.

I guess it is. I expected that you would have used the manifest values as the base, but upon further checking, you didn't.

candita · 2023-10-05T22:33:09Z

/approve

openshift-ci · 2023-10-05T22:33:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita

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

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 · 2023-10-05T22:35:18Z

@Miciah: Jira Issue OCPBUGS-20024: All pull requests linked via external trackers have merged:

openshift/cluster-dns-operator#384

Jira Issue OCPBUGS-20024 has been moved to the MODIFIED state.

In response to this:

The defect in the daemon controller, OCPBUGS-19452 that prevented us from using maxSurge has been fixed by openshift/kubernetes#1716.

This reverts #379.

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.

Followup to PR openshift#384. - pkg/operator/controller/controller_dns_node_resolver_daemonset.go - small update to a comment - pkg/operator/controller/dns_status.go - hardcode maxUnavailable to 10% of desiredNumberScheduled and remove condition "invalid maxUnavailable value" - pkg/operator/controller/dns_status_test.go - remove maxUnavailable format testing

Followup to PR openshift#384. - pkg/operator/controller/controller_dns_node_resolver_daemonset.go - small update to a comment - pkg/operator/controller/dns_status.go - hardcode maxUnavailable to 10% of desiredNumberScheduled - pkg/operator/controller/dns_status_test.go - remove maxUnavailable format testing

Followup to PR openshift#384. - pkg/operator/controller/controller_dns_node_resolver_daemonset.go - small update to a comment - pkg/operator/controller/dns_status.go - hardcode maxUnavailable to 10% of desiredNumberScheduled - pkg/operator/controller/dns_status_test.go - remove maxUnavailable format testing and cleanup noderesolver testing

openshift-merge-robot · 2023-11-01T07:59:42Z

Fix included in accepted release 4.15.0-0.nightly-2023-11-01-040931

Followup to PR openshift#384. - pkg/operator/controller/controller_dns_node_resolver_daemonset.go - small update to a comment - pkg/operator/controller/dns_status.go - hardcode maxUnavailable to 10% of desiredNumberScheduled - pkg/operator/controller/dns_status_test.go - remove maxUnavailable format testing and cleanup noderesolver testing

openshift-merge-robot · 2024-01-05T21:27:45Z

Fix included in accepted release 4.15.0-0.nightly-2024-01-05-151121

openshift-ci bot requested review from melvinjoseph86, frobware and gcs278 October 3, 2023 00:14

Miciah mentioned this pull request Oct 3, 2023

[release-4.13] OCPBUGS-11449: Set DNS DaemonSet's maxSurge value to 10% #366

Merged

openshift-ci bot assigned alebedev87 Oct 4, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2023

candita reviewed Oct 5, 2023

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2023

openshift-ci bot merged commit 5e12334 into openshift:master Oct 5, 2023
9 checks passed

candita mentioned this pull request Oct 12, 2023

OCPBUGS-20024: Ignore max unavailable for status #386

Merged

openshift-ci-robot mentioned this pull request Oct 13, 2023

OCPBUGS-20024: Revert "OCPBUGS-13366: ignore repeated TopologyAwareHintsDisabled events" openshift/origin#28233

Merged

openshift-ci-robot mentioned this pull request Jan 4, 2024

[release-4.15] OCPBUGS-20024: Enable topology-aware hints iff nodes in >=2 zones #399

Merged

Miciah mentioned this pull request Feb 13, 2024

[release-4.14] NO-JIRA: (OCPBUGS 5943) Ignore max unavailable for status #400

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-20024: Revert "Revert "Set DNS DaemonSet's maxSurge value to 10%"" #384

OCPBUGS-20024: Revert "Revert "Set DNS DaemonSet's maxSurge value to 10%"" #384

Miciah commented Oct 3, 2023

openshift-ci-robot commented Oct 3, 2023

openshift-ci-robot commented Oct 3, 2023

openshift-ci bot commented Oct 3, 2023

openshift-merge-robot commented Oct 3, 2023

candita commented Oct 4, 2023

alebedev87 commented Oct 5, 2023

Miciah commented Oct 5, 2023

candita Oct 5, 2023

candita Oct 5, 2023

candita Oct 5, 2023

candita Oct 5, 2023

candita commented Oct 5, 2023

openshift-ci bot commented Oct 5, 2023

openshift-ci-robot commented Oct 5, 2023

openshift-merge-robot commented Nov 1, 2023

openshift-merge-robot commented Jan 5, 2024

OCPBUGS-20024: Revert "Revert "Set DNS DaemonSet's maxSurge value to 10%"" #384

OCPBUGS-20024: Revert "Revert "Set DNS DaemonSet's maxSurge value to 10%"" #384

Conversation

Miciah commented Oct 3, 2023

openshift-ci-robot commented Oct 3, 2023

openshift-ci-robot commented Oct 3, 2023

openshift-ci bot commented Oct 3, 2023

openshift-merge-robot commented Oct 3, 2023

candita commented Oct 4, 2023

alebedev87 commented Oct 5, 2023

Miciah commented Oct 5, 2023

candita Oct 5, 2023

Choose a reason for hiding this comment

candita Oct 5, 2023

Choose a reason for hiding this comment

candita Oct 5, 2023

Choose a reason for hiding this comment

candita Oct 5, 2023

Choose a reason for hiding this comment

candita commented Oct 5, 2023

openshift-ci bot commented Oct 5, 2023

openshift-ci-robot commented Oct 5, 2023

openshift-merge-robot commented Nov 1, 2023

openshift-merge-robot commented Jan 5, 2024