Skip to content

Commit

Permalink
feat: Simply code by removing redundant serverConfig check via annota…
Browse files Browse the repository at this point in the history
…tion
  • Loading branch information
wikoion committed May 16, 2024
1 parent 0550197 commit 998fb1d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 45 deletions.
43 changes: 1 addition & 42 deletions pkg/machinefilters/machine_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package machinefilters

import (
"encoding/json"
"reflect"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -90,11 +89,6 @@ func MatchesKThreesBootstrapConfig(machineConfigs map[string]*bootstrapv1.KThree
return false
}

// Check if KCP and machine KThreesServerConfig matches, if not return
if !matchKThreesServerConfig(kcp, machine) {
return false
}

bootstrapRef := machine.Spec.Bootstrap.ConfigRef
if bootstrapRef == nil {
// Missing bootstrap reference should not be considered as unmatching.
Expand All @@ -110,46 +104,11 @@ func MatchesKThreesBootstrapConfig(machineConfigs map[string]*bootstrapv1.KThree
}

kcpConfig := kcp.Spec.KThreesConfigSpec.DeepCopy()
// KCP bootstrapv1.KThreesServerConfig will only be compared with a machine's ServerConfig annotation, so
// we are cleaning up from the reflect.DeepEqual comparison.
kcpConfig.ServerConfig = bootstrapv1.KThreesServerConfig{}
machineConfig.Spec.ServerConfig = bootstrapv1.KThreesServerConfig{}

// KCP version check is handled elsewhere
kcpConfig.Version = ""
machineConfig.Spec.Version = ""

return reflect.DeepEqual(&machineConfig.Spec, kcpConfig)
}
}

// matchKThreesServerConfig verifies if KCP and machine ClusterConfiguration matches.
// NOTE: Machines that have KThreesServerConfigurationAnnotation will have to match with KCP KThreesServerConfig.
// If the annotation is not present (machine is either old or adopted), we won't roll out on any possible changes
// made in KCP's KThreesServerConfig given that we don't have enough information to make a decision.
// Users should use KCP.Spec.RolloutAfter field to force a rollout in this case.
func matchKThreesServerConfig(kcp *controlplanev1.KThreesControlPlane, machine *clusterv1.Machine) bool {
kThreesServerConfigStr, ok := machine.GetAnnotations()[controlplanev1.KThreesServerConfigurationAnnotation]
if !ok {
// We don't have enough information to make a decision; don't' trigger a roll out.
return true
}

kThreesServerConfig := &bootstrapv1.KThreesServerConfig{}

// ClusterConfiguration annotation is not correct, only solution is to rollout.
// The call to json.Unmarshal has to take a pointer to the pointer struct defined above,
// otherwise we won't be able to handle a nil ClusterConfiguration (that is serialized into "null").
if err := json.Unmarshal([]byte(kThreesServerConfigStr), &kThreesServerConfig); err != nil {
return false
}

// If any of the compared values are nil, treat them the same as an empty KThreesServerConfig.
if kThreesServerConfig == nil {
kThreesServerConfig = &bootstrapv1.KThreesServerConfig{}
}

kcpLocalKThreesServerConfig := &kcp.Spec.KThreesConfigSpec.ServerConfig

// Compare and return.
return reflect.DeepEqual(kThreesServerConfig, kcpLocalKThreesServerConfig)
}
33 changes: 30 additions & 3 deletions pkg/machinefilters/machine_filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,41 @@ func TestMatchesKThreesBootstrapConfig(t *testing.T) {
},
}
m := &clusterv1.Machine{
TypeMeta: metav1.TypeMeta{
Kind: "KThreesConfig",
APIVersion: clusterv1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
controlplanev1.KThreesServerConfigurationAnnotation: "{\n \"clusterDomain\": \"bar\"\n}",
Namespace: "default",
Name: "test",
},
Spec: clusterv1.MachineSpec{
Bootstrap: clusterv1.Bootstrap{
ConfigRef: &corev1.ObjectReference{
Kind: "KThreesConfig",
Namespace: "default",
Name: "test",
APIVersion: bootstrapv1.GroupVersion.String(),
},
},
},
}
machineConfigs := map[string]*bootstrapv1.KThreesConfig{
m.Name: {},
m.Name: {
TypeMeta: metav1.TypeMeta{
Kind: "KThreesConfig",
APIVersion: bootstrapv1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "test",
},
Spec: bootstrapv1.KThreesConfigSpec{
ServerConfig: bootstrapv1.KThreesServerConfig{
ClusterDomain: "bar",
},
},
},
}
match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
g.Expect(match).To(BeFalse())
Expand Down

0 comments on commit 998fb1d

Please sign in to comment.