MCO-595: Remove MCO's pending config workflow

[djoshy]

- What I did
Removed all references/checks for the journal pending config writes. In some cases, I replaced them with desiredConfig checks and in other cases removed them entirely.

- How to verify it
Upgrades tests should work without any new complaints (:

- Description for the changelog
daemon: removed pending config checks

[openshift-ci-robot]

@djoshy: This pull request references MCO-595 which is a valid jira issue.

In response to this:

- What I did
Removed all references/checks for the journal pending config writes. In some cases, I replaced them with desiredConfig checks and in other cases removed them entirely.

- How to verify it
Upgrades tests should work without any new complaints (:

- Description for the changelog
daemon: removed pending config checks

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]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[djoshy]

/test unit
/test verify

[djoshy]

/test e2e-aws-ovn-upgrade

[yuqi-zhang]

/test e2e-gcp-op

[djoshy]

/test unit
/test verify
/test e2e-gcp-op

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-gcp-op

[cgwalters]

There's a lot of subtlety here. I love the goal. The current logic is immensely confusing and hard to understand. (This strongly relates to #1190 )

That said, IIRC the idea with "pending config" is to be a "transient state" between "current" and "desired". I don't see how we can entirely remove that...it seems to me that we at least need a single bit of state that says "we're trying to transition to the desired config"?

[yuqi-zhang]

That said, IIRC the idea with "pending config" is to be a "transient state" between "current" and "desired". I don't see how we can entirely remove that...it seems to me that we at least need a single bit of state that says "we're trying to transition to the desired config"?

So, we do have that already today, done in two ways:

1. a currentconfig file we write to disk, indicating the transient state
2. a journal entry (which this is trying to remove)

And when we revisited the workflow, the two did essentially the exact same thing, so there was a duplication of effort, which was fine when things went well, but if anything errored, the journal entry made it exponentially harder to fix if it was e.g. referring to a non-existent hash, even a forcefile did not remove it.

So we wanted to do either or: 1. have the journal or 2. have the file indicate the transient state. We think 2 is easier to manage and less prone to errors + helps remove the MCO's dependency on journal

Hopefully that makes sense, we will try to make sure that what I just described above isn't broken (no regressions due to the removal of this)

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-gcp-op
/test unit
/test verify

[djoshy]

/test verify

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-aws-ovn-upgrade

[djoshy]

/test e2e-gcp-op

[djoshy]

/test e2e-aws-ovn-upgrade

[djoshy]

/test e2e-aws-ovn-upgrade

[djoshy]

/hold
waiting on #3718 to merge

[djoshy]

/hold cancel
/retest-required

[djoshy]

/test e2e-aws-ovn-upgrade

[sinnykumari]

Putting hold for qe pre-merge testing
/hold

[sinnykumari]

Overall LGTM.
/approve
I am little concerned that this change hitting some corner case regression. But I do agree with Jerry's rationale.
As Jerry has more content of this issue, will be good to get your review as well.

[djoshy]

/retest-required

[sergiordlr]

Verified using IPI on AWS.

We run a whole regression but the hypershift test cases.

All cases passed. We add the qe-approved label

/label qe-approved

[djoshy]

/unhold

/retest-required

[yuqi-zhang]

/lgtm

Thanks for chasing this down David!

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: djoshy, sinnykumari, yuqi-zhang

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 [sinnykumari,yuqi-zhang]

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

[openshift-ci]

@djoshy: 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.