Skip to content

Commit

Permalink
fix: Merge specs deeply
Browse files Browse the repository at this point in the history
  • Loading branch information
zacharyblasczyk committed Sep 3, 2024
1 parent e847258 commit 5ad91a9
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 19 deletions.
21 changes: 16 additions & 5 deletions controllers/weightsandbiases_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,23 +159,34 @@ func (r *WeightsAndBiasesReconciler) Reconcile(ctx context.Context, req ctrl.Req
log.Info("Initial desired spec", "spec", desiredSpec.SensitiveValuesMasked())
}

// First takes precedence
desiredSpec.Merge(crdSpec)
if err := desiredSpec.Merge(crdSpec); err != nil {
log.Error(err, "Failed to merge CRD spec into desired spec")
return ctrlqueue.RequeueWithError(err)
}
if r.Debug {
log.Info("Desired spec after merging crdSpec", "spec", desiredSpec.SensitiveValuesMasked())
}

desiredSpec.Merge(userInputSpec)
if err := desiredSpec.Merge(userInputSpec); err != nil {
log.Error(err, "Failed to merge user input spec into desired spec")
return ctrlqueue.RequeueWithError(err)
}
if r.Debug {
log.Info("Desired spec after merging userInputSpec", "spec", desiredSpec.SensitiveValuesMasked())
}

desiredSpec.Merge(deployerSpec)
if err := desiredSpec.Merge(deployerSpec); err != nil {
log.Error(err, "Failed to merge deployer spec into desired spec")
return ctrlqueue.RequeueWithError(err)
}
if r.Debug {
log.Info("Desired spec after merging deployerSpec", "spec", desiredSpec.SensitiveValuesMasked())
}

desiredSpec.Merge(operator.Defaults(wandb, r.Scheme))
if err := desiredSpec.Merge(operator.Defaults(wandb, r.Scheme)); err != nil {
log.Error(err, "Failed to merge operator defaults into desired spec")
return ctrlqueue.RequeueWithError(err)
}
if r.Debug {
log.Info("Desired spec after merging operator defaults", "spec", desiredSpec.SensitiveValuesMasked())
}
Expand Down
35 changes: 21 additions & 14 deletions pkg/wandb/spec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,26 +88,30 @@ func (s *Spec) IsEqual(spec *Spec) bool {
return isReleaseEqual && isValuesEqual && isMetadataEqual
}

func (s *Spec) mergeConfig(values Values) (err error) {
func (s *Spec) mergeConfig(newValues map[string]interface{}) error {
if s.Values == nil {
s.Values = values
return nil
s.Values = make(map[string]interface{})
}
mergedValues, err := s.Values.Merge(values)
if err != nil {
return err

for key, newValue := range newValues {
if existingValue, exists := s.Values[key]; exists {
if _, ok := existingValue.(map[string]interface{}); ok {
if newMap, ok := newValue.(map[string]interface{}); ok {
if err := s.mergeConfig(newMap); err != nil {
return err
}
continue
}
}
}
s.Values[key] = newValue
}
s.Values = mergedValues
return nil
}

// Merge merges the given spec into the current spec. The currently spec will
// take precedence. This means, if the current spec has a value for a key, the
// new spec will not override it. Likewise, charts & metadata cannot be merged,
// so if a chart/metadata value is set, the one passed in will be ignored.
func (s *Spec) Merge(spec *Spec) {
func (s *Spec) Merge(spec *Spec) error {
if spec == nil {
return
return nil
}

if s.Metadata == nil {
Expand All @@ -117,8 +121,11 @@ func (s *Spec) Merge(spec *Spec) {
s.Chart = spec.Chart
}
if spec.Values != nil {
s.mergeConfig(spec.Values)
if err := s.mergeConfig(spec.Values); err != nil {
return err
}
}
return nil
}

func (s *Spec) SensitiveValuesMasked() *Spec {
Expand Down

0 comments on commit 5ad91a9

Please sign in to comment.