Fast-forward v0 #133
Merged
Fast-forward v0 #133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When recert runs into an error, it returns it as a `Result` from the `main` function. This causes the Rust runtime to print the error trace. However, the error does not get included in the recert summary files. In the summary, you only see the logs, which makes it look like no error occurred. This commit makes it so that when recert runs into an error the error will be included in summary files
Include error in recert summary
Enabled `+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic)]` (but also `allow-unwrap-in-tests = true` because it's too strict for tests) This means that `unwrap`, `expect` and `panic` are not allowed in the project, unless the programmer states `#[allow(clippy::unwrap_used)]`, preferably with a comment explaining why the `unwrap`/`expect` could never panic. Removed all instances of `unwrap` which had better alternatives, but left some that were reasonable. This should stop recert from causing panics (unfortunately dependencies might still panic) Also removed references to `anyhow::Error` from return values, as this is unnecessarily verbose and can be inferred implicitly. It's constantly being added by the rust_analyzer's extract_function code action, and I forget to remove it.
This change ensures that rollouts after recert for the following components are skipped: - openshift-apiserver/openshift-apiserver - openshift-oauth-apiserver/oauth-apiserver The goal is the reduction of the stabilization time of the Ingress and the Authentication cluster operators. Specifically, those rollouts are triggered by the difference in annotations on the respective deployments. The latter include: - a spec-hash (SHA256 of the deployment.spec) annotation - multiple ConfigMap and Secret fnv32 hash annotations of objects that the respective cluster-operators are watching In order to correctly compute the spec-hash annotations, we need to ensure that whatever the respective cluster-operators compute the hash on, is what we use as well. Because the operators include a mix of YAML and Go code for the specs they compute the spec-hash on, we do the same with a hack script written in Go. The latter: - downloads the required deployment YAML manifests from the specific OCP release branch - encodes them in JSON (mainly because JSON encoding between Go and Rust are quite different) - adds some spec fields that can be hardcoded and leaves some templated spec fields to be filled in in Rust code (e.g. container image pull-specs, revisions, etc.). The fnv32 hash annotations are also adjusted to follow the Go JSON encoding implementation: - secret data fields are lexicographically ordered - when a field is a nil pointer it's encoded as the 'null' JSON type Signed-off-by: Michail Resvanis <[email protected]>
We had a too strict condition, see diff comment
Prevents the follow error:
```
Caused by:
0: performing ocp specific post-processing
1: renaming proxy
2: renaming all
3: renaming etcd resources
4: fixing storages
5: no "/kubernetes.io/operator.openshift.io/storages/cluster"
: exit status 1
```
# Background The `cluster-config-v1` configmaps contain a copy of `install-config.yaml` that was used for installation. In that install-config, one of the fields is the machine-network CIDR. The cluster-network-operator reads the machine-network field from the `cluster-config-v1` configmaps while it tries to determine the final value for `noProxy`, so that means we must ensure those configmaps have the current machine network value. # Current support Currently recert supports replacing the entire install-config in those configmaps. This is good for IBU, where we already have a cluster we can take the entire install-config from. # Issue In IBI, we would have to fake such install-config for the fresh cluster. This is inconvenient. # Solution Add a new `--machine-network-cidr` rename option to recert, which would replace just the machine-network-cidr in the existing (seed) install-config, rather than forcing the user to supply the entire machine-config.
MGMT-16777: Compute the hash annotations for {openshift,oauth}-apiserver
Support proxy change in clusters without storage
Don't use `unwrap`
Signed-off-by: Michail Resvanis <[email protected]>
Add support for machine-network-cidr rename
OCPBUGS-32514: Fix recert failing because of non-existent managedFields
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omertuc 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.