Skip to content

Commit d10961f

Browse files
fl64fl64
committed
fix vmclass generic creation logic
Signed-off-by: Pavel Tishkov <[email protected]>
1 parent 6d49229 commit d10961f

File tree

4 files changed

+148
-41
lines changed

4 files changed

+148
-41
lines changed

‎images/hooks/pkg/hooks/create-generic-vmclass/hook.go‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -87,26 +87,43 @@ func Reconcile(_ context.Context, input *pkg.HookInput) error {
8787
moduleStateSecrets := input.Snapshots.Get(moduleStateSecretSnapshot)
8888
vmClasses := input.Snapshots.Get(vmClassSnapshot)
8989

90+
vmClassExists := len(vmClasses) > 0
9091
shouldCreateVMClass := false
91-
if len(moduleStateSecrets) > 0 {
92+
93+
if len(moduleStateSecrets) == 0 {
94+
// Секрет отсутствует - создаем VMClass если его нет
95+
shouldCreateVMClass = !vmClassExists
96+
input.Logger.Info("Module-state secret doesn't exist, will create generic VirtualMachineClass if it doesn't exist")
97+
} else {
98+
// Секрет существует - проверяем его содержимое
9299
moduleStateData := make(map[string]interface{})
93100
if err := moduleStateSecrets[0].UnmarshalTo(&moduleStateData); err == nil {
94101
if genericCreatedEncoded, exists := moduleStateData["generic-vmclass-created"]; exists {
95102
if encodedStr, ok := genericCreatedEncoded.(string); ok {
96103
if decodedBytes, err := base64.StdEncoding.DecodeString(encodedStr); err == nil {
97-
if string(decodedBytes) == "true" {
104+
genericCreated := string(decodedBytes) == "true"
105+
if !genericCreated && !vmClassExists {
106+
shouldCreateVMClass = true
107+
input.Logger.Info("Module-state indicates generic VirtualMachineClass was not created, creating it")
108+
} else if genericCreated && vmClassExists {
109+
input.Logger.Info("Module-state correctly reflects that generic VirtualMachineClass exists")
110+
} else if genericCreated && !vmClassExists {
111+
input.Logger.Info("Module-state indicates generic VirtualMachineClass was created but it doesn't exist, will recreate it")
98112
shouldCreateVMClass = true
99-
input.Logger.Info("Found record in module-state that generic VirtualMachineClass was created previously")
113+
} else {
114+
input.Logger.Info("Module-state correctly reflects that generic VirtualMachineClass doesn't exist")
100115
}
101116
}
102117
}
118+
} else {
119+
// Ключ отсутствует в секрете - создаем VMClass если его нет
120+
shouldCreateVMClass = !vmClassExists
121+
input.Logger.Info("Module-state secret doesn't contain generic-vmclass-created key, will create generic VirtualMachineClass if it doesn't exist")
103122
}
104123
}
105124
}
106125

107-
vmClassExists := len(vmClasses) > 0
108-
109-
if shouldCreateVMClass && !vmClassExists {
126+
if shouldCreateVMClass {
110127
input.Logger.Info("Creating generic VirtualMachineClass")
111128

112129
vmClass := &v1alpha2.VirtualMachineClass{
@@ -166,11 +183,7 @@ func Reconcile(_ context.Context, input *pkg.HookInput) error {
166183
}
167184

168185
input.PatchCollector.Create(vmClass)
169-
input.Logger.Info("Generic VirtualMachineClass creation requested")
170-
} else if shouldCreateVMClass && vmClassExists {
171-
input.Logger.Info("Generic VirtualMachineClass already exists, no action needed")
172-
} else if !shouldCreateVMClass && !vmClassExists {
173-
input.Logger.Info("No record of generic VirtualMachineClass creation in module-state, skipping creation")
186+
input.Logger.Info("VirtualMachineClass generic created")
174187
}
175188

176189
return nil

‎images/hooks/pkg/hooks/create-generic-vmclass/hook_test.go‎

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ var _ = Describe("Create Generic VMClass hook", func() {
7575
})
7676
})
7777

78-
It("should create generic vmclass when it doesn't exist", func() {
78+
It("should recreate generic vmclass when it doesn't exist but state says it was created", func() {
7979
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{})
8080

8181
patchCollector.CreateMock.Set(func(obj interface{}) {
@@ -109,9 +109,28 @@ var _ = Describe("Create Generic VMClass hook", func() {
109109
snapshots.GetMock.When(moduleStateSecretSnapshot).Then([]pkg.Snapshot{})
110110
})
111111

112-
It("should not create generic vmclass", func() {
112+
It("should create generic vmclass when it doesn't exist", func() {
113113
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{})
114114

115+
patchCollector.CreateMock.Set(func(obj interface{}) {
116+
vmClass, ok := obj.(*v1alpha2.VirtualMachineClass)
117+
Expect(ok).To(BeTrue())
118+
Expect(vmClass.Name).To(Equal("generic"))
119+
Expect(vmClass.Labels).To(Equal(map[string]string{
120+
"app": "virtualization-controller",
121+
"module": "virtualization",
122+
}))
123+
})
124+
125+
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
126+
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(1))
127+
})
128+
129+
It("should not create generic vmclass when it already exists", func() {
130+
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{
131+
mock.NewSnapshotMock(GinkgoT()),
132+
})
133+
115134
patchCollector.CreateMock.Optional()
116135

117136
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
@@ -135,9 +154,73 @@ var _ = Describe("Create Generic VMClass hook", func() {
135154
})
136155
})
137156

138-
It("should not create generic vmclass", func() {
157+
It("should create generic vmclass when it doesn't exist", func() {
158+
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{})
159+
160+
patchCollector.CreateMock.Set(func(obj interface{}) {
161+
vmClass, ok := obj.(*v1alpha2.VirtualMachineClass)
162+
Expect(ok).To(BeTrue())
163+
Expect(vmClass.Name).To(Equal("generic"))
164+
Expect(vmClass.Labels).To(Equal(map[string]string{
165+
"app": "virtualization-controller",
166+
"module": "virtualization",
167+
}))
168+
})
169+
170+
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
171+
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(1))
172+
})
173+
174+
It("should not create generic vmclass when it already exists", func() {
175+
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{
176+
mock.NewSnapshotMock(GinkgoT()),
177+
})
178+
179+
patchCollector.CreateMock.Optional()
180+
181+
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
182+
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(0))
183+
})
184+
})
185+
186+
Context("when module-state secret exists with generic-vmclass-created=false", func() {
187+
BeforeEach(func() {
188+
moduleStateData := map[string]interface{}{
189+
"generic-vmclass-created": base64.StdEncoding.EncodeToString([]byte("false")),
190+
}
191+
192+
snapshots.GetMock.When(moduleStateSecretSnapshot).Then([]pkg.Snapshot{
193+
mock.NewSnapshotMock(GinkgoT()).UnmarshalToMock.Set(func(v any) error {
194+
data, ok := v.(*map[string]interface{})
195+
Expect(ok).To(BeTrue())
196+
*data = moduleStateData
197+
return nil
198+
}),
199+
})
200+
})
201+
202+
It("should create generic vmclass when it doesn't exist", func() {
139203
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{})
140204

205+
patchCollector.CreateMock.Set(func(obj interface{}) {
206+
vmClass, ok := obj.(*v1alpha2.VirtualMachineClass)
207+
Expect(ok).To(BeTrue())
208+
Expect(vmClass.Name).To(Equal("generic"))
209+
Expect(vmClass.Labels).To(Equal(map[string]string{
210+
"app": "virtualization-controller",
211+
"module": "virtualization",
212+
}))
213+
})
214+
215+
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
216+
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(1))
217+
})
218+
219+
It("should not create generic vmclass when it already exists", func() {
220+
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{
221+
mock.NewSnapshotMock(GinkgoT()),
222+
})
223+
141224
patchCollector.CreateMock.Optional()
142225

143226
Expect(Reconcile(context.Background(), newInput())).To(Succeed())

‎images/hooks/pkg/hooks/update-module-state/hook.go‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -115,41 +115,39 @@ func Reconcile(_ context.Context, input *pkg.HookInput) error {
115115
vmClassExists := len(vmClasses) > 0
116116

117117
needsUpdate := false
118-
currentState := false
118+
hasBeenCreated := false
119119

120120
if len(moduleStateSecrets) > 0 {
121121
moduleStateData := make(map[string]interface{})
122122
if err := moduleStateSecrets[0].UnmarshalTo(&moduleStateData); err == nil {
123123
if genericCreatedEncoded, exists := moduleStateData[genericVMClassStateKey]; exists {
124124
if encodedStr, ok := genericCreatedEncoded.(string); ok {
125125
if decodedBytes, err := base64.StdEncoding.DecodeString(encodedStr); err == nil {
126-
currentState = string(decodedBytes) == "true"
126+
hasBeenCreated = string(decodedBytes) == "true"
127127
}
128128
}
129129
}
130130
}
131131
}
132132

133-
if vmClassExists && !currentState {
133+
// Записываем в секрет только когда VMClass создан и еще не записано
134+
if vmClassExists && !hasBeenCreated {
134135
needsUpdate = true
135-
input.Logger.Info("Generic VirtualMachineClass exists but module-state doesn't reflect this, updating secret")
136-
} else if !vmClassExists && currentState {
137-
input.Logger.Info("Generic VirtualMachineClass doesn't exist but module-state indicates it was created previously - keeping historical record")
138-
} else if len(moduleStateSecrets) == 0 {
139-
needsUpdate = true
140-
input.Logger.Info("Module-state secret doesn't exist, creating it")
141-
} else if vmClassExists && currentState {
142-
input.Logger.Info("Module-state correctly reflects that generic VirtualMachineClass exists")
143-
} else {
144-
input.Logger.Info("Module-state correctly reflects that generic VirtualMachineClass doesn't exist")
136+
input.Logger.Info("Generic VirtualMachineClass exists but module-state doesn't reflect it was created, updating secret")
137+
} else if vmClassExists && hasBeenCreated {
138+
input.Logger.Info("Module-state correctly reflects that generic VirtualMachineClass was created")
139+
} else if !vmClassExists && hasBeenCreated {
140+
input.Logger.Info("Generic VirtualMachineClass was created previously but doesn't exist now - keeping historical record")
141+
} else if !vmClassExists && !hasBeenCreated {
142+
input.Logger.Info("Generic VirtualMachineClass doesn't exist and was never created - no action needed")
145143
}
146144

147145
if needsUpdate {
148-
state := ModuleState{GenericVMClassCreated: vmClassExists}
146+
state := ModuleState{GenericVMClassCreated: true} // Всегда записываем true, когда VMClass существует
149147

150148
if len(moduleStateSecrets) > 0 {
151149
input.PatchCollector.PatchWithMerge(state.ToPatchData(), "v1", "Secret", settings.ModuleNamespace, moduleStateSecretName)
152-
input.Logger.Info("Updated module-state secret")
150+
input.Logger.Info("Updated module-state secret to record that generic VirtualMachineClass was created")
153151
} else {
154152
secret := &corev1.Secret{
155153
TypeMeta: metav1.TypeMeta{
@@ -167,7 +165,7 @@ func Reconcile(_ context.Context, input *pkg.HookInput) error {
167165
Type: "Opaque",
168166
}
169167
input.PatchCollector.Create(secret)
170-
input.Logger.Info("Created module-state secret")
168+
input.Logger.Info("Created module-state secret to record that generic VirtualMachineClass was created")
171169
}
172170
}
173171

‎images/hooks/pkg/hooks/update-module-state/hook_test.go‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -149,29 +149,42 @@ var _ = Describe("Update Module State hook", func() {
149149
snapshots.GetMock.When(vmClassSnapshot).Then([]pkg.Snapshot{})
150150
})
151151

152-
It("should create module-state secret with false value when it doesn't exist", func() {
152+
It("should not create module-state secret when vmclass doesn't exist and secret doesn't exist", func() {
153153
snapshots.GetMock.When(moduleStateSecretSnapshot).Then([]pkg.Snapshot{})
154154

155-
patchCollector.CreateMock.Set(func(obj interface{}) {
156-
secret, ok := obj.(*corev1.Secret)
157-
Expect(ok).To(BeTrue())
158-
Expect(secret.Name).To(Equal("module-state"))
159-
Expect(secret.Namespace).To(Equal(settings.ModuleNamespace))
160-
Expect(secret.Data).To(HaveKey("generic-vmclass-created"))
155+
patchCollector.CreateMock.Optional()
156+
patchCollector.PatchWithMergeMock.Optional()
161157

162-
Expect(string(secret.Data["generic-vmclass-created"])).To(Equal("false"))
158+
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
159+
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(0))
160+
Expect(patchCollector.PatchWithMergeMock.Calls()).To(HaveLen(0))
161+
})
162+
163+
It("should keep historical record when vmclass doesn't exist but module-state indicates it was created", func() {
164+
moduleStateData := map[string]interface{}{
165+
"generic-vmclass-created": base64.StdEncoding.EncodeToString([]byte("true")),
166+
}
167+
168+
snapshots.GetMock.When(moduleStateSecretSnapshot).Then([]pkg.Snapshot{
169+
mock.NewSnapshotMock(GinkgoT()).UnmarshalToMock.Set(func(v any) error {
170+
data, ok := v.(*map[string]interface{})
171+
Expect(ok).To(BeTrue())
172+
*data = moduleStateData
173+
return nil
174+
}),
163175
})
164176

165177
patchCollector.PatchWithMergeMock.Optional()
178+
patchCollector.CreateMock.Optional()
166179

167180
Expect(Reconcile(context.Background(), newInput())).To(Succeed())
168-
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(1))
169181
Expect(patchCollector.PatchWithMergeMock.Calls()).To(HaveLen(0))
182+
Expect(patchCollector.CreateMock.Calls()).To(HaveLen(0))
170183
})
171184

172-
It("should keep historical record when vmclass doesn't exist but module-state indicates it was created", func() {
185+
It("should not update module-state secret when vmclass doesn't exist and secret contains false", func() {
173186
moduleStateData := map[string]interface{}{
174-
"generic-vmclass-created": base64.StdEncoding.EncodeToString([]byte("true")),
187+
"generic-vmclass-created": base64.StdEncoding.EncodeToString([]byte("false")),
175188
}
176189

177190
snapshots.GetMock.When(moduleStateSecretSnapshot).Then([]pkg.Snapshot{

0 commit comments

Comments
 (0)