daemon.go/update.go: various cleanup surrounding OS updates #2973

mkenigs · 2022-02-28T18:51:18Z

Originally merged into mcbs in #2896

IsCoreOSVariant and compareOSImageURL have already been called or equivalent checks have been performed for all cases updateOS is called Since updateOS no longer requires any members of Daemon, it can be made a helper function instead of a method on Daemon

Call IsCoreOSVariant once in applyOSChanges instead of in every helper function

The comment on the function says it's probably unnecessary, and it adds unnecessary complexity to logic that must be maintained in two separate OS update paths (one in update() and one in checkStateOnFirstRun())

Certain helper methods should only be called on CoreOS, and it is more reliable to type check this than rely on method preconditions

cgwalters · 2022-02-28T19:16:14Z

/lgtm

openshift-ci · 2022-02-28T19:16:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, mkenigs

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:

~~OWNERS~~ [cgwalters]

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

openshift-bot · 2022-02-28T19:18:54Z

/retest-required

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

openshift-bot · 2022-02-28T19:31:51Z

/retest-required

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

openshift-bot · 2022-02-28T21:02:52Z

/retest-required

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

openshift-bot · 2022-02-28T21:15:52Z

/retest-required

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

cgwalters · 2022-02-28T22:09:50Z

Looked at e2e-aws-workers-rhel8:

fatal: [ip-10-0-196-76.us-west-2.compute.internal]: FAILED! => {
    "changed": false, 
    "msg": "As of v4.10, RHEL nodes must be at least version 8.4"
}

Not related to this PR for sure. Hmm, and can't we just remove e2e-aws-workers-rhel7?

openshift-bot · 2022-02-28T22:46:54Z

/retest-required

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

openshift-bot · 2022-02-28T23:51:54Z

/retest-required

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

openshift-bot · 2022-03-01T01:15:54Z

/retest-required

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

openshift-bot · 2022-03-01T01:27:52Z

/retest-required

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

openshift-bot · 2022-03-01T02:16:31Z

/retest-required

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

openshift-bot · 2022-03-01T04:03:53Z

/retest-required

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

openshift-bot · 2022-03-01T04:27:55Z

/retest-required

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

openshift-bot · 2022-03-01T05:15:52Z

/retest-required

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

openshift-bot · 2022-03-01T05:51:54Z

/retest-required

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

openshift-bot · 2022-03-01T06:51:55Z

/retest-required

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

openshift-bot · 2022-03-01T07:15:52Z

/retest-required

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

openshift-bot · 2022-03-01T08:03:54Z

/retest-required

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

openshift-bot · 2022-03-01T08:27:53Z

/retest-required

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

openshift-bot · 2022-03-01T08:39:54Z

/retest-required

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

cgwalters · 2022-03-09T13:31:14Z

/hold cancel

openshift-bot · 2022-03-09T13:31:25Z

/retest-required

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

openshift-bot · 2022-03-09T14:31:41Z

/retest-required

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

openshift-bot · 2022-03-09T14:55:26Z

/retest-required

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

openshift-bot · 2022-03-09T15:19:27Z

/retest-required

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

openshift-bot · 2022-03-09T15:31:26Z

/retest-required

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

openshift-bot · 2022-03-09T18:19:28Z

/retest-required

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

openshift-bot · 2022-03-09T18:31:25Z

/retest-required

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

openshift-bot · 2022-03-09T19:07:27Z

/retest-required

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

openshift-bot · 2022-03-09T21:09:28Z

/retest-required

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

openshift-bot · 2022-03-09T23:29:26Z

/retest-required

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

openshift-bot · 2022-03-10T00:08:26Z

/retest-required

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

openshift-bot · 2022-03-10T02:18:25Z

/retest-required

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

openshift-bot · 2022-03-10T03:23:27Z

/retest-required

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

openshift-bot · 2022-03-10T05:07:25Z

/retest-required

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

openshift-bot · 2022-03-10T05:33:26Z

/retest-required

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

openshift-bot · 2022-03-10T05:46:26Z

/retest-required

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

openshift-bot · 2022-03-10T09:18:28Z

/retest-required

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

openshift-bot · 2022-03-10T10:36:30Z

/retest-required

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

openshift-bot · 2022-03-10T13:51:29Z

/retest-required

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

openshift-ci · 2022-03-10T14:14:51Z

@mkenigs: 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/e2e-ovn-step-registry	`a5bd3e5`	link	false	`/test e2e-ovn-step-registry`
ci/prow/e2e-metal-ipi	`a5bd3e5`	link	false	`/test e2e-metal-ipi`
ci/prow/e2e-aws-serial	`a5bd3e5`	link	false	`/test e2e-aws-serial`
ci/prow/e2e-aws-workers-rhel8	`a5bd3e5`	link	false	`/test e2e-aws-workers-rhel8`
ci/prow/e2e-aws-workers-rhel7	`a5bd3e5`	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-upgrade-single-node	`a5bd3e5`	link	false	`/test e2e-aws-upgrade-single-node`
ci/prow/e2e-aws-single-node	`a5bd3e5`	link	false	`/test e2e-aws-single-node`
ci/prow/e2e-aws-disruptive	`a5bd3e5`	link	false	`/test e2e-aws-disruptive`
ci/prow/e2e-gcp-upgrade	`a5bd3e5`	link	false	`/test e2e-gcp-upgrade`

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.

cgwalters · 2022-03-10T15:23:01Z

OK looking at the history here, we do have one green for e2e-agnostic-upgrade: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/2973/pull-ci-openshift-machine-config-operator-master-e2e-agnostic-upgrade/1501666514103701504

The failures there are a mix of unrelated provisioning issues, and Prometheus availabilty (not finding an existing BZ for this). I am inclined to override.

cgwalters · 2022-03-10T16:01:48Z

/override e2e-agnostic-upgrade

cgwalters · 2022-03-10T16:02:08Z

/skip

openshift-ci · 2022-03-10T16:06:39Z

@cgwalters: /override requires a failed status context or a job name to operate on.
The following unknown contexts were given:

e2e-agnostic-upgrade

Only the following contexts were expected:

ci/prow/bootstrap-unit
ci/prow/e2e-agnostic-upgrade
ci/prow/e2e-aws
ci/prow/e2e-aws-disruptive
ci/prow/e2e-aws-serial
ci/prow/e2e-aws-single-node
ci/prow/e2e-aws-upgrade-single-node
ci/prow/e2e-aws-workers-rhel7
ci/prow/e2e-aws-workers-rhel8
ci/prow/e2e-gcp-op
ci/prow/e2e-gcp-op-single-node
ci/prow/e2e-gcp-upgrade
ci/prow/e2e-metal-ipi
ci/prow/e2e-ovn-step-registry
ci/prow/e2e-vsphere-upgrade
ci/prow/images
ci/prow/okd-e2e-aws
ci/prow/okd-images
ci/prow/unit
ci/prow/verify
pull-ci-openshift-machine-config-operator-fcos-e2e-aws
pull-ci-openshift-machine-config-operator-fcos-images
pull-ci-openshift-machine-config-operator-fcos-unit
pull-ci-openshift-machine-config-operator-fcos-verify
pull-ci-openshift-machine-config-operator-layering-e2e-gcp-op
pull-ci-openshift-machine-config-operator-layering-e2e-gcp-op-single-node
pull-ci-openshift-machine-config-operator-master-bootstrap-unit
pull-ci-openshift-machine-config-operator-master-e2e-agnostic-upgrade
pull-ci-openshift-machine-config-operator-master-e2e-aws-disruptive
pull-ci-openshift-machine-config-operator-master-e2e-aws-serial
pull-ci-openshift-machine-config-operator-master-e2e-aws-single-node
pull-ci-openshift-machine-config-operator-master-e2e-aws-upgrade-single-node
pull-ci-openshift-machine-config-operator-master-e2e-aws-workers-rhel8
pull-ci-openshift-machine-config-operator-master-e2e-gcp-upgrade
pull-ci-openshift-machine-config-operator-master-e2e-metal-ipi
pull-ci-openshift-machine-config-operator-master-e2e-ovn-step-registry
pull-ci-openshift-machine-config-operator-master-e2e-vsphere-upgrade
pull-ci-openshift-machine-config-operator-master-okd-e2e-aws
pull-ci-openshift-machine-config-operator-master-okd-images
pull-ci-openshift-machine-config-operator-release-4.6-e2e-aws-workers-rhel7
tide

Details

In response to this:

/override e2e-agnostic-upgrade

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.

cgwalters · 2022-03-10T16:08:01Z

/override ci/prow/e2e-agnostic-upgrade

openshift-ci · 2022-03-10T16:10:22Z

@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/e2e-agnostic-upgrade

Details

In response to this:

/override ci/prow/e2e-agnostic-upgrade

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.

mkenigs added 4 commits February 28, 2022 12:47

Consolidate IsCoreOSVariant calls

8e30e61

Call IsCoreOSVariant once in applyOSChanges instead of in every helper function

Remove compareOSImageURL

f5552dc

The comment on the function says it's probably unnecessary, and it adds unnecessary complexity to logic that must be maintained in two separate OS update paths (one in update() and one in checkStateOnFirstRun())

Add CoreOSDaemon struct

a5bd3e5

Certain helper methods should only be called on CoreOS, and it is more reliable to type check this than rely on method preconditions

openshift-ci bot requested review from cgwalters and sinnykumari February 28, 2022 18:51

openshift-ci bot assigned cgwalters Feb 28, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 28, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2022

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2022

openshift-merge-robot merged commit 8982b90 into openshift:master Mar 10, 2022

mkenigs deleted the os-update-cleanup branch March 18, 2022 13:48

daemon.go/update.go: various cleanup surrounding OS updates #2973

daemon.go/update.go: various cleanup surrounding OS updates #2973

Uh oh!

Conversation

mkenigs commented Feb 28, 2022

Uh oh!

cgwalters commented Feb 28, 2022

Uh oh!

openshift-ci bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

cgwalters commented Feb 28, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

openshift-bot commented Mar 1, 2022

Uh oh!

cgwalters commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 9, 2022

Uh oh!

openshift-bot commented Mar 10, 2022

Uh oh!

openshift-bot commented Mar 10, 2022

Uh oh!

openshift-bot commented Mar 10, 2022

Uh oh!

openshift-bot commented Mar 10, 2022

Uh oh!

openshift-bot commented Mar 10, 2022

Uh oh!

openshift-ci bot commented Mar 10, 2022 •

edited

Loading