From 63d0f018d2dbfac36f58c1556056e544e135596e Mon Sep 17 00:00:00 2001 From: Adam Janikowski <12255597+ajanikow@users.noreply.github.com> Date: Tue, 29 Mar 2022 12:51:11 +0200 Subject: [PATCH] [Bugfix] Allow nil architecture (#943) --- CHANGELOG.md | 1 + pkg/apis/deployment/v1/architecture.go | 8 ++++++ pkg/apis/deployment/v2alpha1/architecture.go | 8 ++++++ pkg/deployment/images.go | 2 +- pkg/deployment/member/phase_updates.go | 26 ++++++++++++------- pkg/deployment/pod/affinity.go | 2 +- .../reconcile/action_member_phase_update.go | 2 +- pkg/deployment/resources/pod_creator.go | 2 +- .../resources/pod_creator_arangod.go | 2 +- pkg/deployment/resources/pod_creator_sync.go | 2 +- 10 files changed, 40 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0056061f..411c91afa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - (Bugfix) Skip Replace operation on DBServer if they need to be scaled down - (Feature) Upgrade procedure steps - (Refactor) Remove API and Core cross-dependency +- (Bugfix) Allow to have nil architecture (NPE fix) ## [1.2.8](https://github.com/arangodb/kube-arangodb/tree/1.2.8) (2022-02-24) - Do not check License V2 on Community images diff --git a/pkg/apis/deployment/v1/architecture.go b/pkg/apis/deployment/v1/architecture.go index 7151417cf..c3a32127f 100644 --- a/pkg/apis/deployment/v1/architecture.go +++ b/pkg/apis/deployment/v1/architecture.go @@ -72,6 +72,14 @@ func (a ArangoDeploymentArchitectureType) Validate() error { } } +func (a *ArangoDeploymentArchitectureType) Default(def ArangoDeploymentArchitectureType) ArangoDeploymentArchitectureType { + if a == nil { + return def + } + + return *a +} + func (a ArangoDeploymentArchitectureType) AsNodeSelectorRequirement() core.NodeSelectorTerm { return core.NodeSelectorTerm{ MatchExpressions: []core.NodeSelectorRequirement{ diff --git a/pkg/apis/deployment/v2alpha1/architecture.go b/pkg/apis/deployment/v2alpha1/architecture.go index 5939d64fd..f5d9ee4e2 100644 --- a/pkg/apis/deployment/v2alpha1/architecture.go +++ b/pkg/apis/deployment/v2alpha1/architecture.go @@ -72,6 +72,14 @@ func (a ArangoDeploymentArchitectureType) Validate() error { } } +func (a *ArangoDeploymentArchitectureType) Default(def ArangoDeploymentArchitectureType) ArangoDeploymentArchitectureType { + if a == nil { + return def + } + + return *a +} + func (a ArangoDeploymentArchitectureType) AsNodeSelectorRequirement() core.NodeSelectorTerm { return core.NodeSelectorTerm{ MatchExpressions: []core.NodeSelectorRequirement{ diff --git a/pkg/deployment/images.go b/pkg/deployment/images.go index f30ca5d59..5d6a1da1c 100644 --- a/pkg/deployment/images.go +++ b/pkg/deployment/images.go @@ -357,7 +357,7 @@ func (i *ImageUpdatePod) GetNodeAffinity() *core.NodeAffinity { a := core.NodeAffinity{} arch := i.spec.Architecture.GetDefault() - pod.AppendArchSelector(&a, &arch) + pod.AppendArchSelector(&a, arch) pod.MergeNodeAffinity(&a, i.spec.ID.Get().NodeAffinity) diff --git a/pkg/deployment/member/phase_updates.go b/pkg/deployment/member/phase_updates.go index 0e511b969..8eee9b3da 100644 --- a/pkg/deployment/member/phase_updates.go +++ b/pkg/deployment/member/phase_updates.go @@ -32,12 +32,12 @@ const ( recentTerminationsKeepPeriod = time.Minute * 30 ) -type phaseMapFunc func(obj meta.Object, group api.ServerGroup, action api.Action, m *api.MemberStatus) +type phaseMapFunc func(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, action api.Action, m *api.MemberStatus) type phaseMapTo map[api.MemberPhase]phaseMapFunc type phaseMap map[api.MemberPhase]phaseMapTo type PhaseExecutor interface { - Execute(obj meta.Object, group api.ServerGroup, m *api.MemberStatus, action api.Action, to api.MemberPhase) bool + Execute(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, m *api.MemberStatus, action api.Action, to api.MemberPhase) bool } func GetPhaseExecutor() PhaseExecutor { @@ -46,22 +46,30 @@ func GetPhaseExecutor() PhaseExecutor { var phase = phaseMap{ api.MemberPhaseNone: { - api.MemberPhasePending: func(obj meta.Object, group api.ServerGroup, action api.Action, m *api.MemberStatus) { + api.MemberPhasePending: func(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, action api.Action, m *api.MemberStatus) { // Change member RID m.RID = uuid.NewUUID() // Clean Pod details m.PodUID = "" - m.ClusterID = obj.GetUID() + // Add ClusterID + if m.ClusterID == "" { + m.ClusterID = obj.GetUID() + } + + if m.Architecture == nil { + d := spec.Architecture.GetDefault() + m.Architecture = &d + } }, }, api.MemberPhasePending: { - api.MemberPhaseCreated: func(obj meta.Object, group api.ServerGroup, action api.Action, m *api.MemberStatus) { + api.MemberPhaseCreated: func(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, action api.Action, m *api.MemberStatus) { // Clean conditions removeMemberConditionsMapFunc(m) }, - api.MemberPhaseUpgrading: func(obj meta.Object, group api.ServerGroup, action api.Action, m *api.MemberStatus) { + api.MemberPhaseUpgrading: func(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, action api.Action, m *api.MemberStatus) { removeMemberConditionsMapFunc(m) }, }, @@ -94,7 +102,7 @@ func removeMemberConditionsMapFunc(m *api.MemberStatus) { m.Upgrade = false } -func (p phaseMap) empty(obj meta.Object, group api.ServerGroup, action api.Action, m *api.MemberStatus) { +func (p phaseMap) empty(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, action api.Action, m *api.MemberStatus) { } @@ -108,7 +116,7 @@ func (p phaseMap) getFunc(from, to api.MemberPhase) phaseMapFunc { return p.empty } -func (p phaseMap) Execute(obj meta.Object, group api.ServerGroup, m *api.MemberStatus, action api.Action, to api.MemberPhase) bool { +func (p phaseMap) Execute(obj meta.Object, spec api.DeploymentSpec, group api.ServerGroup, m *api.MemberStatus, action api.Action, to api.MemberPhase) bool { from := m.Phase if from == to { @@ -119,7 +127,7 @@ func (p phaseMap) Execute(obj meta.Object, group api.ServerGroup, m *api.MemberS m.Phase = to - f(obj, group, action, m) + f(obj, spec, group, action, m) return true } diff --git a/pkg/deployment/pod/affinity.go b/pkg/deployment/pod/affinity.go index d433fcbbf..597242a30 100644 --- a/pkg/deployment/pod/affinity.go +++ b/pkg/deployment/pod/affinity.go @@ -51,7 +51,7 @@ func AppendPodAntiAffinityDefault(p interfaces.PodCreator, a *core.PodAntiAffini } } -func AppendArchSelector(a *core.NodeAffinity, arch *api.ArangoDeploymentArchitectureType) { +func AppendArchSelector(a *core.NodeAffinity, arch api.ArangoDeploymentArchitectureType) { if a.RequiredDuringSchedulingIgnoredDuringExecution == nil { a.RequiredDuringSchedulingIgnoredDuringExecution = &core.NodeSelector{} } diff --git a/pkg/deployment/reconcile/action_member_phase_update.go b/pkg/deployment/reconcile/action_member_phase_update.go index 0a1db3c6e..2cbff4f73 100644 --- a/pkg/deployment/reconcile/action_member_phase_update.go +++ b/pkg/deployment/reconcile/action_member_phase_update.go @@ -72,7 +72,7 @@ func (a *memberPhaseUpdateAction) Start(ctx context.Context) (bool, error) { return true, nil } - if member.GetPhaseExecutor().Execute(a.actionCtx.GetAPIObject(), a.action.Group, &m, a.action, p) { + if member.GetPhaseExecutor().Execute(a.actionCtx.GetAPIObject(), a.actionCtx.GetSpec(), a.action.Group, &m, a.action, p) { if err := a.actionCtx.UpdateMember(ctx, m); err != nil { return false, errors.WithStack(err) } diff --git a/pkg/deployment/resources/pod_creator.go b/pkg/deployment/resources/pod_creator.go index bb57cf62e..d056d4e8f 100644 --- a/pkg/deployment/resources/pod_creator.go +++ b/pkg/deployment/resources/pod_creator.go @@ -554,7 +554,7 @@ func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspect m.PodSpecVersion = template.PodSpecChecksum } - member.GetPhaseExecutor().Execute(r.context.GetAPIObject(), group, &m, api.Action{}, newPhase) + member.GetPhaseExecutor().Execute(r.context.GetAPIObject(), spec, group, &m, api.Action{}, newPhase) if top := status.Topology; top.Enabled() { if m.Topology != nil && m.Topology.ID == top.ID { diff --git a/pkg/deployment/resources/pod_creator_arangod.go b/pkg/deployment/resources/pod_creator_arangod.go index e2a9ca9c4..424baf521 100644 --- a/pkg/deployment/resources/pod_creator_arangod.go +++ b/pkg/deployment/resources/pod_creator_arangod.go @@ -332,7 +332,7 @@ func (m *MemberArangoDPod) GetPodAffinity() *core.PodAffinity { func (m *MemberArangoDPod) GetNodeAffinity() *core.NodeAffinity { a := core.NodeAffinity{} - pod.AppendArchSelector(&a, m.status.Architecture) + pod.AppendArchSelector(&a, m.status.Architecture.Default(m.spec.Architecture.GetDefault())) pod.MergeNodeAffinity(&a, m.groupSpec.NodeAffinity) diff --git a/pkg/deployment/resources/pod_creator_sync.go b/pkg/deployment/resources/pod_creator_sync.go index 12e3e8dfe..bba2afa11 100644 --- a/pkg/deployment/resources/pod_creator_sync.go +++ b/pkg/deployment/resources/pod_creator_sync.go @@ -231,7 +231,7 @@ func (m *MemberSyncPod) GetPodAffinity() *core.PodAffinity { func (m *MemberSyncPod) GetNodeAffinity() *core.NodeAffinity { a := core.NodeAffinity{} - pod.AppendArchSelector(&a, m.memberStatus.Architecture) + pod.AppendArchSelector(&a, m.memberStatus.Architecture.Default(m.spec.Architecture.GetDefault())) pod.MergeNodeAffinity(&a, m.groupSpec.NodeAffinity)