[deckhouse-controller] New HookInput Snapshot logic [second-iteration] #13941
Conversation
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
Signed-off-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
f1d52eb to
57e453b
Compare
Signed-off-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
Signed-off-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
Signed-off-by: Michail Sinelnikov <72449391+RottenRat@users.noreply.github.com>
added 3 commits
June 18, 2025 18:51
Signed-off-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
Signed-off-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
ldmonster
requested changes
Jun 23, 2025
| } | ||
| legacyMode := snaps[0] | ||
| // legacyMode is set in the provider cluster configuration secret | ||
| input.Values.Set("cloudProviderVcd.internal.legacyMode", legacyMode) |
There was a problem hiding this comment.
only if legacy mode is true
Suggested change
| input.Values.Set("cloudProviderVcd.internal.legacyMode", legacyMode) | |
| input.Values.Set("cloudProviderVcd.internal.legacyMode", legacyMode) |
| for node, err := range sdkobjectpatch.SnapshotIter[nodeKernelVersion](nodes) { | ||
| if err != nil { | ||
| input.Logger.Error("failed to iterate over 'nodes' snapshots", log.Err(err)) | ||
| continue |
There was a problem hiding this comment.
do not continue here. it gives a panic in previous realization. make return
| @@ -165,9 +174,10 @@ func handleDiscoveryDataVolumeTypes( | |||
| } | |||
| } | |||
|
|
|||
| storageClassSnapshots := make(map[string]*storage.StorageClass) | |||
| // TODO: review this, looks like deadcode | |||
| storageClassSnapshots := make(map[string]storage.StorageClass) | |||
| for _, snapshot := range input.Snapshots["storage_classes"] { | |||
There was a problem hiding this comment.
@RottenRat refactor this please
| for endpoint, err := range sdkobjectpatch.SnapshotIter[etcdInstance](etcdEndpointsSnapshots) { | ||
| if err != nil { | ||
| input.Logger.Error("failed to iterate over 'etcd_endpoints' snapshot", log.Err(err)) | ||
| currentQuotaBytes = defaultEtcdMaxSize |
There was a problem hiding this comment.
you must return error here. where did you find this logic?
| pod := spod.(controlPlaneManagerPod) | ||
| for pod, err := range sdkobjectpatch.SnapshotIter[controlPlaneManagerPod](podsSnaps) { | ||
| if err != nil { | ||
| input.Logger.Error("failed to iterate over 'cpm_pods' snapshots", log.Err(err)) |
There was a problem hiding this comment.
return error here, because of casting
|
|
||
| if len(conditions) == 0 { | ||
| if len(conditionsSnaps) == 0 { |
There was a problem hiding this comment.
we must check len(conditions) here and len(conditionsSnaps) before
| for _, item := range nodeUserSnap { | ||
| nuForClear := item.(nodeUsersForClear) | ||
| var nuForClear nodeUsersForClear |
| @@ -68,9 +72,12 @@ func updateControlPlane(input *go_hook.HookInput) error { | |||
| "externalManagedControlPlane": true, | |||
| }, | |||
| } | |||
| for controlPlane, err := range sdkobjectpatch.SnapshotIter[controlPlane](input.NewSnapshots.Get("control_plane")) { | |||
| if err != nil { | |||
| input.Logger.Error("failed to iterate over 'control_plane' classes", log.Err(err)) | |||
There was a problem hiding this comment.
return error because of casting
| @@ -50,7 +50,7 @@ var _ = sdk.RegisterFunc(&go_hook.HookConfig{ | |||
| }, discoverCloudProviderHandler) | |||
|
|
|||
| func discoverCloudProviderHandler(input *go_hook.HookInput) error { | |||
| secret := input.Snapshots["cloud_provider_secret"] | |||
| secret := input.NewSnapshots.Get("cloud_provider_secret") | |||
There was a problem hiding this comment.
this is an error.
you must check your PR for this errors.
if you set to values raw snapshot - it will be nested not correct.
try it local
| @@ -65,8 +65,8 @@ metadata: | |||
|
|
|||
| It("`nodeManager.internal.cloudProvider must be filled with data from secret", func() { | |||
| Expect(f).To(ExecuteSuccessfully()) | |||
| Expect(f.ValuesGet("nodeManager.internal.cloudProvider.b64String").String()).To(Equal("abc")) | |||
| Expect(f.ValuesGet("nodeManager.internal.cloudProvider.b64JSON.parse").String()).To(Equal("me")) | |||
| Expect(f.ValuesGet("nodeManager.internal.cloudProvider.Wrapped.b64String").String()).To(Equal("abc")) | |||
There was a problem hiding this comment.
it will be not wrapped. fix this
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.
Description
Second iteration of New Snapshot logic
Refactored access to snapshots by replacing direct indexing (input.Snapshots["snapshot_name"]) with method calls via the new field NewSnapshots. Access to snapshots is now performed using the .Get() method. For example, instead of casting like
input.Snapshots["snapshot_name"][0].(NodeInfo)we use:WARNING
!!We ignore all nil results in new snapshots realization!!!
Additionally, two new methods provided by the library
sdkobjectpatch "github.com/deckhouse/module-sdk/pkg/object-patch"have been introduced:policy, ok := policySnap.(EgressGatewayPolicyInfo); if !ok { continue }.Why do we need it, and what problem does it solve?
The previous approach relied heavily on unsafe type assertions and direct index accesses which led to potential runtime panics and inconsistencies. By switching to structured methods and proper error handling, the refactoring reduces the risk of bugs caused by incorrect types or missing elements, thus improving stability and maintainability of the codebase.
Checklist
Changelog entries