Bump internal ignition spec version to v3.6.0 and ignition/v2 dependency to 2.26.0#6236
Draft
SpaceFace02 wants to merge 2 commits into
Draft
Bump internal ignition spec version to v3.6.0 and ignition/v2 dependency to 2.26.0#6236SpaceFace02 wants to merge 2 commits into
SpaceFace02 wants to merge 2 commits into
Conversation
Update MCO's internal ignition spec from v3.5 to v3.6, add v3.5<->v3.6 conversion paths, and bump the ignition/v2 dependency to v2.26.0. Signed-off-by: Chirag Rao <crao@redhat.com> Co-authored-by: Fangge Jin <fjin@redhat.com>
Run go mod tidy && go mod vendor to sync the vendor directory with the updated go.mod after bumping ignition/v2 to v2.26.0. Signed-off-by: Chirag Rao <crao@redhat.com>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughThe PR updates the codebase to Ignition v3.6/3.6.0 across module pins, constants, converters, server handling, controllers, daemons, and tests. ChangesIgnition v3.6 rollout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
needs-ok-to-test
Contributor
|
Hi @SpaceFace02. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SpaceFace02 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Author
|
/test all |
Contributor
|
@SpaceFace02: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn 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 kubernetes-sigs/prow repository. |
Author
|
Pre-requisite for openshift/installer#10392 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pkg/server/api_test.go (1)
446-447: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the
v3.5handler scenario alongside the newv3.6case.Replacing the old
v3.5request withv3.6drops end-to-end coverage for the still-supported compatibility path that now relies on the newv3.6 -> 3.5conversion. Please add thev3.6scenario without removing the existingv3.5one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/server/api_test.go` around lines 446 - 447, The API test case currently replaces the existing v3.5 request with a v3.6 request, which removes coverage for the supported compatibility path. Update the table-driven test in api_test.go so the existing v3.5 handler scenario remains in place and add a separate v3.6 case alongside it, using the relevant request setup helpers such as setV3_5AcceptHeaderOnReq and setV3_6AcceptHeaderOnReq to keep both paths covered.pkg/controller/common/helpers_test.go (1)
200-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new
3.5 -> 3.6conversion edge.This table only exercises the new downgrade path (
3.6 -> 3.5). A broken or missing3.5 -> 3.6registration inbuildConverterList()would still pass here. Please add a direct3.5 -> 3.6case, or change one transitive case to target3.6.0, so both new directions are covered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/common/helpers_test.go` around lines 200 - 210, The conversion tests in helpers_test should also cover the new upgrade edge from 3.5 to 3.6, not just the existing 3.6 to 3.5 downgrade. Add a direct table case using the relevant input config and version pair in the conversion matrix, or retarget one existing transitive case to 3.6.0 so buildConverterList() is validated in both directions.pkg/daemon/daemon_test.go (1)
13-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a v3.6-specific mode-bit test here.
TestValidateFilesstill derives modes withfi.Mode().Perm(), so this upgrade only exercises the low permission bits and won't catch regressions in the new v3.6 setuid/setgid/sticky-bit support called out in the PR. Please add a case that passes one of those special bits throughcheckV3Files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/daemon/daemon_test.go` at line 13, `TestValidateFiles` currently only checks permission bits via `fi.Mode().Perm()`, so it does not cover the new v3.6 special mode bits. Add a v3.6-specific test case in `daemon_test.go` that exercises `checkV3Files` with a mode containing one of the special bits such as setuid, setgid, or sticky bit, using the existing `ign3types` v3.6 types to locate the right path and validate the new behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended-priv/mco_storage.go`:
- Around line 284-289: Update the hard-coded default Ignition version in the OTP
deployment test to match the new default used elsewhere. In the test that
declares allVersions and defaultIgnitionVersion, change the expected value from
the old 3.5.0 default to the new default so the rendered MachineConfig assertion
in this scenario aligns with pkg/controller/common/constants.go and
test/extended-priv/const.go. Use the existing symbols allVersions and
defaultIgnitionVersion in that test to locate the change.
---
Nitpick comments:
In `@pkg/controller/common/helpers_test.go`:
- Around line 200-210: The conversion tests in helpers_test should also cover
the new upgrade edge from 3.5 to 3.6, not just the existing 3.6 to 3.5
downgrade. Add a direct table case using the relevant input config and version
pair in the conversion matrix, or retarget one existing transitive case to 3.6.0
so buildConverterList() is validated in both directions.
In `@pkg/daemon/daemon_test.go`:
- Line 13: `TestValidateFiles` currently only checks permission bits via
`fi.Mode().Perm()`, so it does not cover the new v3.6 special mode bits. Add a
v3.6-specific test case in `daemon_test.go` that exercises `checkV3Files` with a
mode containing one of the special bits such as setuid, setgid, or sticky bit,
using the existing `ign3types` v3.6 types to locate the right path and validate
the new behavior.
In `@pkg/server/api_test.go`:
- Around line 446-447: The API test case currently replaces the existing v3.5
request with a v3.6 request, which removes coverage for the supported
compatibility path. Update the table-driven test in api_test.go so the existing
v3.5 handler scenario remains in place and add a separate v3.6 case alongside
it, using the relevant request setup helpers such as setV3_5AcceptHeaderOnReq
and setV3_6AcceptHeaderOnReq to keep both paths covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d5e4b072-3679-41b9-bda1-27cd771cba25
⛔ Files ignored due to path filters (122)
go.sumis excluded by!**/*.sumvendor/github.com/aws/aws-sdk-go-v2/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/NOTICE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/arn/arn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/dbus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/methods.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/dbus/subscription_set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/deserialize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/unit/serialize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v23tov30/v23tov30.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v32tov22/v32tov22.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v32tov31/v32tov31.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v33tov32/v33tov32.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v34tov33/v34tov33.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v35tov34/v35tov34.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/translate/v36tov35/v36tov35.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ign-converter/util/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/shared/errors/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_4/translate/translate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_4/types/directory.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_4/types/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_4/types/mode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_4/types/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_5/types/directory.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_5/types/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_5/types/mode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_5/types/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/translate/translate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/cex.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/clevis.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/device.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/directory.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/disk.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/filesystem.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/headers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/ignition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/kargs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/luks.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/mode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/node.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/partition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/passwd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/path.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/raid.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/resource.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/schema.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/storage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/systemd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/tang.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/tls.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/unit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/url.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/v3_6/types/verification.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/ignition/v2/config/validate/validate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/flag.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal/parse.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/metadata_supplier.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/semconv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.37.0/rpcconv/metric.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/print.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/read.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/modfile/rule.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/module/module.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc9218.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/websocket/hybi.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/term/terminal.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/time/rate/rate.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/time/rate/sometimes.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/ctrlflow/ctrlflow.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/doc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/printf.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/analysis/passes/printf/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/astutil/imports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ssa/create.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/typeutil/callee.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/astutil/util.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/core/export.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/label/label.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/imports/sortimports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/modindex/lookup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/moreiters/iters.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/delete.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/edit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/imports.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/refactor/refactor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/stdlib.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/classify_call.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/versions/features.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/refactor/satisfy/find.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (57)
devex/cmd/mcdiff/diff.godevex/cmd/onclustertesting/internal/legacycmds/machineconfig.gogo.modlib/resourceapply/machineconfig_test.gopkg/controller/bootstrap/bootstrap_test.gopkg/controller/bootstrap/testdata/bootstrap/99_openshift-machineconfig_master.yamlpkg/controller/bootstrap/testdata/bootstrap/99_openshift-machineconfig_worker.yamlpkg/controller/build/fixtures/objects.gopkg/controller/common/constants.gopkg/controller/common/helpers.gopkg/controller/common/helpers_test.gopkg/controller/common/ignition_converter.gopkg/controller/common/reconcile.gopkg/controller/common/reconcile_test.gopkg/controller/container-runtime-config/container_runtime_config_controller.gopkg/controller/container-runtime-config/container_runtime_config_controller_test.gopkg/controller/container-runtime-config/helpers.gopkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.gopkg/controller/kubelet-config/helpers.gopkg/controller/kubelet-config/kubelet_config_controller.gopkg/controller/kubelet-config/kubelet_config_controller_test.gopkg/controller/kubelet-config/kubelet_config_features_test.gopkg/controller/kubelet-config/kubelet_config_nodes_test.gopkg/controller/render/render_controller_test.gopkg/controller/template/render_test.gopkg/daemon/config_drift_monitor.gopkg/daemon/config_drift_monitor_test.gopkg/daemon/daemon.gopkg/daemon/daemon_test.gopkg/daemon/drain.gopkg/daemon/drain_test.gopkg/daemon/file_writers.gopkg/daemon/on_disk_validation.gopkg/daemon/pinned_image_set.gopkg/daemon/runtimeassets/revertservice.gopkg/daemon/runtimeassets/revertservice_test.gopkg/daemon/runtimeassets/runtimeassets.gopkg/daemon/update.gopkg/daemon/update_test.gopkg/server/api.gopkg/server/api_test.gopkg/server/cluster_server.gopkg/server/server.gopkg/server/server_test.gotest/e2e-1of2/mcd_test.gotest/e2e-2of2/mcc_test.gotest/e2e-2of2/nodedisrupt_test.gotest/e2e-2of2/osimageurl_override_test.gotest/e2e-ocl-2of2/onclusterlayering_test.gotest/e2e-ocl-shared/helpers.gotest/e2e-shared-tests/mcd_config_drift.gotest/e2e-single-node/sno_mcd_test.gotest/e2e-techpreview/osimagestreamrender_test.gotest/extended-priv/const.gotest/extended-priv/mco_storage.gotest/helpers/helpers.gotest/helpers/utils.go
Comment on lines
+284
to
289
| g.It("[PolarionID:63477][OTP] Deploy files using all available ignition configs. Default 3.6.0[Disruptive]", func() { | ||
| var ( | ||
| wMcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker) | ||
| mcNames = "mc-tc-63477" | ||
| allVersions = []string{"2.2.0", "3.0.0", "3.1.0", "3.2.0", "3.3.0", "3.4.0", "3.5.0"} | ||
| allVersions = []string{"2.2.0", "3.0.0", "3.1.0", "3.2.0", "3.3.0", "3.4.0", "3.5.0", "3.6.0"} | ||
| defaultIgnitionVersion = "3.5.0" // default version is 3.5.0 for OCP > 4.19 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the expected default Ignition version here too.
Line 289 still hard-codes "3.5.0", so this test now contradicts both its own title and the new defaults in pkg/controller/common/constants.go and test/extended-priv/const.go. That leaves the rendered-MC assertion checking the pre-upgrade value.
Suggested fix
- defaultIgnitionVersion = "3.5.0" // default version is 3.5.0 for OCP > 4.19
+ defaultIgnitionVersion = IgnitionDefaultVersion📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| g.It("[PolarionID:63477][OTP] Deploy files using all available ignition configs. Default 3.6.0[Disruptive]", func() { | |
| var ( | |
| wMcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker) | |
| mcNames = "mc-tc-63477" | |
| allVersions = []string{"2.2.0", "3.0.0", "3.1.0", "3.2.0", "3.3.0", "3.4.0", "3.5.0"} | |
| allVersions = []string{"2.2.0", "3.0.0", "3.1.0", "3.2.0", "3.3.0", "3.4.0", "3.5.0", "3.6.0"} | |
| defaultIgnitionVersion = "3.5.0" // default version is 3.5.0 for OCP > 4.19 | |
| g.It("[PolarionID:63477][OTP] Deploy files using all available ignition configs. Default 3.6.0[Disruptive]", func() { | |
| var ( | |
| wMcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker) | |
| mcNames = "mc-tc-63477" | |
| allVersions = []string{"2.2.0", "3.0.0", "3.1.0", "3.2.0", "3.3.0", "3.4.0", "3.5.0", "3.6.0"} | |
| defaultIgnitionVersion = IgnitionDefaultVersion |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended-priv/mco_storage.go` around lines 284 - 289, Update the
hard-coded default Ignition version in the OTP deployment test to match the new
default used elsewhere. In the test that declares allVersions and
defaultIgnitionVersion, change the expected value from the old 3.5.0 default to
the new default so the rendered MachineConfig assertion in this scenario aligns
with pkg/controller/common/constants.go and test/extended-priv/const.go. Use the
existing symbols allVersions and defaultIgnitionVersion in that test to locate
the change.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
- What I did
Bump MCO's internal Ignition spec version from v3.5 to v3.6 and update the
ignition/v2dependency from v2.20.0 to v2.26.0.InternalMCOIgnitionVersionto3.6.0ConvertRawExtIgnitionToV3_5->ConvertRawExtIgnitionToV3_6ign3typesimports fromv3_5tov3_6across all packagesIgnition spec 3.6.0 adds:
modefield on files and directories now respects setuid, setgid, and sticky bitsclevis luks bind- How to verify it
go build ./...compiles successfullymake test-unit(one pre-existing flaky test inTestOSBuildControlleris unrelated)- Description for the changelog
Bump internal Ignition spec from v3.5 to v3.6, enabling support for special mode bits and arbitrary custom Clevis pins.
Summary by CodeRabbit
New Features
Bug Fixes
Chores