fix(machine): Remove link from store if already deleted #832
Conversation
craciunoiuc
force-pushed
the
craciunoiuc/remove-missing-links
branch
3 times, most recently
from
2c78437 to
68a2551
Compare
| embeddedStore, err := store.NewEmbeddedStore[networkv1alpha1.NetworkSpec, networkv1alpha1.NetworkStatus]( | ||
| filepath.Join( | ||
| config.G[config.KraftKit](ctx).RuntimeDir, | ||
| "networkv1alpha1", | ||
| ), | ||
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var existingNetwork networkv1alpha1.Network | ||
| err = embeddedStore.Get(ctx, network.Name, storage.GetOptions{}, &existingNetwork) | ||
| if err != nil && !strings.Contains(err.Error(), "not found") { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
The store is already wrapping the v1alpha1 implementation, meaning that this obfuscates its use.
There was a problem hiding this comment.
I think the wrapping itself makes everything feel obfuscated, and it's fine. As long as it works correctly >:D
nderjung
left a comment
nderjung
left a comment
There was a problem hiding this comment.
In fact the Delete method should not be updated. The Get/List method should be checked to ensure they only return valid entries from the host.
|
Ok then the fix would be to remove from the store on list? |
|
It will still fail if you run |
|
When func (service *v1alpha1Network) List(ctx context.Context, networks *networkv1alpha1.NetworkList) (*networkv1alpha1.NetworkList, error) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~The value of The result The store essentially provides us with "enriched" metadata about items. |
|
Ok I can try to fix there. Hopefully the store is also updated by the returned value, otherwise things will never be removed there |
craciunoiuc
force-pushed
the
craciunoiuc/remove-missing-links
branch
3 times, most recently
from
29c6884 to
68a2551
Compare
Signed-off-by: Cezar Craciunoiu <[email protected]>
craciunoiuc
force-pushed
the
craciunoiuc/remove-missing-links
branch
from
68a2551 to
8d77118
Compare
Signed-off-by: Cezar Craciunoiu <[email protected]>
craciunoiuc
force-pushed
the
craciunoiuc/remove-missing-links
branch
from
8d77118 to
cf6c78b
Compare
|
Converting to draft as not really ready The bug is still there by the way |
2 participants
Prerequisite checklist
make fmton your commit series before opening this PR;Description of changes
This fixes the case where someone creates a network with kraftkit and removes it with something else like
ifconfig/ip link deland the entry stays in the list.Partial fix for: #820