diff --git a/CHANGELOG.md b/CHANGELOG.md index 8edbe3b93..96e9d79dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Add v2alpha1 API for ArangoDeployment and ArangoDeploymentReplication - Migrate CRD to apiextensions.k8s.io/v1 - Add customizable log levels per service +- Move Upgrade as InitContainer and fix Direct Image discovery mode ## [1.1.2](https://github.com/arangodb/kube-arangodb/tree/1.1.2) (2020-11-11) - Fix Bootstrap phase and move it under Plan diff --git a/Makefile b/Makefile index 36ea08ef8..0ad3908ba 100644 --- a/Makefile +++ b/Makefile @@ -618,6 +618,7 @@ set-deployment-api-version-v1: set-api-version/deployment set-api-version/replic set-api-version/%: @grep -rHn "github.com/arangodb/kube-arangodb/pkg/apis/$*/v[A-Za-z0-9]\+" \ "$(ROOT)/pkg/deployment/" \ + "$(ROOT)/pkg/replication/" \ "$(ROOT)/pkg/operator/" \ "$(ROOT)/pkg/server/" \ "$(ROOT)/pkg/util/" \ @@ -627,10 +628,28 @@ set-api-version/%: | xargs -n 1 sed -i "s#github.com/arangodb/kube-arangodb/pkg/apis/$*/v[A-Za-z0-9]\+#github.com/arangodb/kube-arangodb/pkg/apis/$*/v$(API_VERSION)#g" @grep -rHn "DatabaseV[A-Za-z0-9]\+()" \ "$(ROOT)/pkg/deployment/" \ + "$(ROOT)/pkg/replication/" \ "$(ROOT)/pkg/operator/" \ "$(ROOT)/pkg/server/" \ "$(ROOT)/pkg/util/" \ "$(ROOT)/pkg/backup/" \ "$(ROOT)/pkg/apis/backup/" \ | cut -d ':' -f 1 | sort | uniq \ - | xargs -n 1 sed -i "s#DatabaseV[A-Za-z0-9]\+()\.#DatabaseV$(API_VERSION)().#g" \ No newline at end of file + | xargs -n 1 sed -i "s#DatabaseV[A-Za-z0-9]\+()\.#DatabaseV$(API_VERSION)().#g" + @grep -rHn "ReplicationV[A-Za-z0-9]\+()" \ + "$(ROOT)/pkg/deployment/" \ + "$(ROOT)/pkg/replication/" \ + "$(ROOT)/pkg/operator/" \ + "$(ROOT)/pkg/server/" \ + "$(ROOT)/pkg/util/" \ + "$(ROOT)/pkg/backup/" \ + "$(ROOT)/pkg/apis/backup/" \ + | cut -d ':' -f 1 | sort | uniq \ + | xargs -n 1 sed -i "s#ReplicationV[A-Za-z0-9]\+()\.#ReplicationV$(API_VERSION)().#g" + +synchronize-v2alpha1-with-v1: + @rm -f pkg/apis/deployment/v1/zz_generated.deepcopy.go pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go + @for file in $$(find "$(ROOT)/pkg/apis/deployment/v1/" -type f -exec basename {} \;); do cat "$(ROOT)/pkg/apis/deployment/v1/$${file}" | sed "s#package v1#package v2alpha1#g" | sed 's#ArangoDeploymentVersion = "v1"#ArangoDeploymentVersion = "v2alpha1"#g' > "$(ROOT)/pkg/apis/deployment/v2alpha1/$${file}"; done + @make update-generated + @make set-deployment-api-version-v2alpha1 bin + @make set-deployment-api-version-v1 bin \ No newline at end of file diff --git a/pkg/apis/deployment/v1/deployment_spec.go b/pkg/apis/deployment/v1/deployment_spec.go index 6283011c9..a58084def 100644 --- a/pkg/apis/deployment/v1/deployment_spec.go +++ b/pkg/apis/deployment/v1/deployment_spec.go @@ -60,6 +60,8 @@ type DeploymentSpec struct { DowntimeAllowed *bool `json:"downtimeAllowed,omitempty"` DisableIPv6 *bool `json:"disableIPv6,omitempty"` + Upgrade *DeploymentUpgradeSpec `json:"upgrade,omitempty"` + Features *DeploymentFeatures `json:"features,omitempty"` NetworkAttachedVolumes *bool `json:"networkAttachedVolumes,omitempty"` diff --git a/pkg/apis/deployment/v1/deployment_upgrade_spec.go b/pkg/apis/deployment/v1/deployment_upgrade_spec.go new file mode 100644 index 000000000..1269c6a38 --- /dev/null +++ b/pkg/apis/deployment/v1/deployment_upgrade_spec.go @@ -0,0 +1,36 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package v1 + +type DeploymentUpgradeSpec struct { + // Flag specify if upgrade should be auto-injected, even if is not required (in case of stuck) + AutoUpgrade bool `json:"autoUpgrade"` +} + +func (d *DeploymentUpgradeSpec) Get() DeploymentUpgradeSpec { + if d == nil { + return DeploymentUpgradeSpec{} + } + + return *d +} diff --git a/pkg/apis/deployment/v1/image_info.go b/pkg/apis/deployment/v1/image_info.go index ae6c61764..3730080f8 100644 --- a/pkg/apis/deployment/v1/image_info.go +++ b/pkg/apis/deployment/v1/image_info.go @@ -35,6 +35,10 @@ type ImageInfo struct { // ImageInfoList is a list of image infos type ImageInfoList []ImageInfo +func (l ImageInfoList) Add(i ...ImageInfo) ImageInfoList { + return append(l, i...) +} + // GetByImage returns the info in the given list for the image with given name. // If not found, false is returned. func (l ImageInfoList) GetByImage(image string) (ImageInfo, bool) { diff --git a/pkg/apis/deployment/v1/member_status.go b/pkg/apis/deployment/v1/member_status.go index d8c75582a..0e81e000a 100644 --- a/pkg/apis/deployment/v1/member_status.go +++ b/pkg/apis/deployment/v1/member_status.go @@ -69,6 +69,8 @@ type MemberStatus struct { ImageID string `json:"image-id,omitempty"` // Image holds image details Image *ImageInfo `json:"image,omitempty"` + // Upgrade define if upgrade should be enforced during next execution + Upgrade bool `json:"upgrade,omitempty"` } // Equal checks for equality @@ -84,7 +86,8 @@ func (s MemberStatus) Equal(other MemberStatus) bool { reflect.DeepEqual(s.SideCarSpecs, other.SideCarSpecs) && s.ArangoVersion == other.ArangoVersion && s.ImageID == other.ImageID && - s.Image.Equal(other.Image) + s.Image.Equal(other.Image) && + s.Upgrade == other.Upgrade } // Age returns the duration since the creation timestamp of this member. diff --git a/pkg/apis/deployment/v1/server_group_init_containers.go b/pkg/apis/deployment/v1/server_group_init_containers.go index 81397fd33..6e35f5540 100644 --- a/pkg/apis/deployment/v1/server_group_init_containers.go +++ b/pkg/apis/deployment/v1/server_group_init_containers.go @@ -32,11 +32,12 @@ import ( const ( ServerGroupReservedInitContainerNameLifecycle = "init-lifecycle" ServerGroupReservedInitContainerNameUUID = "uuid" + ServerGroupReservedInitContainerNameUpgrade = "upgrade" ) func IsReservedServerGroupInitContainerName(name string) bool { switch name { - case ServerGroupReservedInitContainerNameLifecycle, ServerGroupReservedInitContainerNameUUID: + case ServerGroupReservedInitContainerNameLifecycle, ServerGroupReservedInitContainerNameUUID, ServerGroupReservedInitContainerNameUpgrade: return true default: return false diff --git a/pkg/apis/deployment/v1/zz_generated.deepcopy.go b/pkg/apis/deployment/v1/zz_generated.deepcopy.go index 8b944a8f5..5ceefa6a9 100644 --- a/pkg/apis/deployment/v1/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v1/zz_generated.deepcopy.go @@ -364,6 +364,11 @@ func (in *DeploymentSpec) DeepCopyInto(out *DeploymentSpec) { *out = new(bool) **out = **in } + if in.Upgrade != nil { + in, out := &in.Upgrade, &out.Upgrade + *out = new(DeploymentUpgradeSpec) + **out = **in + } if in.Features != nil { in, out := &in.Features, &out.Features *out = new(DeploymentFeatures) @@ -679,6 +684,22 @@ func (in *DeploymentStatusMembers) DeepCopy() *DeploymentStatusMembers { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeploymentUpgradeSpec) DeepCopyInto(out *DeploymentUpgradeSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeploymentUpgradeSpec. +func (in *DeploymentUpgradeSpec) DeepCopy() *DeploymentUpgradeSpec { + if in == nil { + return nil + } + out := new(DeploymentUpgradeSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalAccessSpec) DeepCopyInto(out *ExternalAccessSpec) { *out = *in diff --git a/pkg/apis/deployment/v2alpha1/deployment_spec.go b/pkg/apis/deployment/v2alpha1/deployment_spec.go index 12950dc24..712785f50 100644 --- a/pkg/apis/deployment/v2alpha1/deployment_spec.go +++ b/pkg/apis/deployment/v2alpha1/deployment_spec.go @@ -60,6 +60,8 @@ type DeploymentSpec struct { DowntimeAllowed *bool `json:"downtimeAllowed,omitempty"` DisableIPv6 *bool `json:"disableIPv6,omitempty"` + Upgrade *DeploymentUpgradeSpec `json:"upgrade,omitempty"` + Features *DeploymentFeatures `json:"features,omitempty"` NetworkAttachedVolumes *bool `json:"networkAttachedVolumes,omitempty"` diff --git a/pkg/apis/deployment/v2alpha1/deployment_upgrade_spec.go b/pkg/apis/deployment/v2alpha1/deployment_upgrade_spec.go new file mode 100644 index 000000000..b3792cd34 --- /dev/null +++ b/pkg/apis/deployment/v2alpha1/deployment_upgrade_spec.go @@ -0,0 +1,36 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package v2alpha1 + +type DeploymentUpgradeSpec struct { + // Flag specify if upgrade should be auto-injected, even if is not required (in case of stuck) + AutoUpgrade bool `json:"autoUpgrade"` +} + +func (d *DeploymentUpgradeSpec) Get() DeploymentUpgradeSpec { + if d == nil { + return DeploymentUpgradeSpec{} + } + + return *d +} diff --git a/pkg/apis/deployment/v2alpha1/image_info.go b/pkg/apis/deployment/v2alpha1/image_info.go index 036b81a22..d7842c331 100644 --- a/pkg/apis/deployment/v2alpha1/image_info.go +++ b/pkg/apis/deployment/v2alpha1/image_info.go @@ -35,6 +35,10 @@ type ImageInfo struct { // ImageInfoList is a list of image infos type ImageInfoList []ImageInfo +func (l ImageInfoList) Add(i ...ImageInfo) ImageInfoList { + return append(l, i...) +} + // GetByImage returns the info in the given list for the image with given name. // If not found, false is returned. func (l ImageInfoList) GetByImage(image string) (ImageInfo, bool) { diff --git a/pkg/apis/deployment/v2alpha1/member_status.go b/pkg/apis/deployment/v2alpha1/member_status.go index 679f64f95..1eef0bb4b 100644 --- a/pkg/apis/deployment/v2alpha1/member_status.go +++ b/pkg/apis/deployment/v2alpha1/member_status.go @@ -69,6 +69,8 @@ type MemberStatus struct { ImageID string `json:"image-id,omitempty"` // Image holds image details Image *ImageInfo `json:"image,omitempty"` + // Upgrade define if upgrade should be enforced during next execution + Upgrade bool `json:"upgrade,omitempty"` } // Equal checks for equality @@ -84,7 +86,8 @@ func (s MemberStatus) Equal(other MemberStatus) bool { reflect.DeepEqual(s.SideCarSpecs, other.SideCarSpecs) && s.ArangoVersion == other.ArangoVersion && s.ImageID == other.ImageID && - s.Image.Equal(other.Image) + s.Image.Equal(other.Image) && + s.Upgrade == other.Upgrade } // Age returns the duration since the creation timestamp of this member. diff --git a/pkg/apis/deployment/v2alpha1/server_group_init_containers.go b/pkg/apis/deployment/v2alpha1/server_group_init_containers.go index 6eee4f8ce..3ae4bd3a5 100644 --- a/pkg/apis/deployment/v2alpha1/server_group_init_containers.go +++ b/pkg/apis/deployment/v2alpha1/server_group_init_containers.go @@ -32,11 +32,12 @@ import ( const ( ServerGroupReservedInitContainerNameLifecycle = "init-lifecycle" ServerGroupReservedInitContainerNameUUID = "uuid" + ServerGroupReservedInitContainerNameUpgrade = "upgrade" ) func IsReservedServerGroupInitContainerName(name string) bool { switch name { - case ServerGroupReservedInitContainerNameLifecycle, ServerGroupReservedInitContainerNameUUID: + case ServerGroupReservedInitContainerNameLifecycle, ServerGroupReservedInitContainerNameUUID, ServerGroupReservedInitContainerNameUpgrade: return true default: return false diff --git a/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go b/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go index 125a4691b..be6df492c 100644 --- a/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go @@ -364,6 +364,11 @@ func (in *DeploymentSpec) DeepCopyInto(out *DeploymentSpec) { *out = new(bool) **out = **in } + if in.Upgrade != nil { + in, out := &in.Upgrade, &out.Upgrade + *out = new(DeploymentUpgradeSpec) + **out = **in + } if in.Features != nil { in, out := &in.Features, &out.Features *out = new(DeploymentFeatures) @@ -679,6 +684,22 @@ func (in *DeploymentStatusMembers) DeepCopy() *DeploymentStatusMembers { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeploymentUpgradeSpec) DeepCopyInto(out *DeploymentUpgradeSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeploymentUpgradeSpec. +func (in *DeploymentUpgradeSpec) DeepCopy() *DeploymentUpgradeSpec { + if in == nil { + return nil + } + out := new(DeploymentUpgradeSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalAccessSpec) DeepCopyInto(out *ExternalAccessSpec) { *out = *in diff --git a/pkg/deployment/reconcile/action_upgrade_member.go b/pkg/deployment/reconcile/action_upgrade_member.go index 0b5a0d00c..d8aa2fd59 100644 --- a/pkg/deployment/reconcile/action_upgrade_member.go +++ b/pkg/deployment/reconcile/action_upgrade_member.go @@ -112,17 +112,12 @@ func (a *actionUpgradeMember) CheckProgress(ctx context.Context) (bool, bool, er log = log.With(). Str("pod-name", m.PodName). Bool("is-upgrading", isUpgrading).Logger() - if !m.Conditions.IsTrue(api.ConditionTypeTerminated) { + if !m.Conditions.IsTrue(api.ConditionTypeReady) { // Pod is not yet terminated return false, false, nil } - // Pod is terminated, we can now remove it - log.Debug().Msg("Deleting pod") - if err := a.actionCtx.DeletePod(m.PodName); err != nil { - return false, false, maskAny(err) - } // Pod is now gone, update the member status - m.Phase = api.MemberPhaseNone + m.Phase = api.MemberPhaseCreated m.RecentTerminations = nil // Since we're upgrading, we do not care about old terminations. m.CleanoutJobID = "" if err := a.actionCtx.UpdateMember(m); err != nil { diff --git a/pkg/deployment/reconcile/plan_builder.go b/pkg/deployment/reconcile/plan_builder.go index de2ea302b..b9d3db248 100644 --- a/pkg/deployment/reconcile/plan_builder.go +++ b/pkg/deployment/reconcile/plan_builder.go @@ -50,6 +50,8 @@ type upgradeDecision struct { UpgradeNeeded bool // If set, the image version has changed UpgradeAllowed bool // If set, it is an allowed version change AutoUpgradeNeeded bool // If set, the database must be started with `--database.auto-upgrade` once + + Hold bool } // CreatePlan considers the current specification & status of the deployment creates a plan to diff --git a/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go b/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go index 9b8d7ccac..10fd5d352 100644 --- a/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go +++ b/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go @@ -90,7 +90,11 @@ func createRotateOrUpgradePlanInternal(log zerolog.Logger, apiObject k8sutil.API } // Got pod, compare it with what it should be - decision := podNeedsUpgrading(log, pod, spec, status.Images) + decision := podNeedsUpgrading(log, m, spec, status.Images) + if decision.Hold { + return nil + } + if decision.UpgradeNeeded && !decision.UpgradeAllowed { // Oops, upgrade is not allowed upgradeNotAllowed = true @@ -108,7 +112,7 @@ func createRotateOrUpgradePlanInternal(log zerolog.Logger, apiObject k8sutil.API if decision.UpgradeNeeded { // Yes, upgrade is needed (and allowed) - newPlan = createUpgradeMemberPlan(log, m, group, "Version upgrade", spec.GetImage(), status, + newPlan = createUpgradeMemberPlan(log, m, group, "Version upgrade", spec, status, !decision.AutoUpgradeNeeded) } else { // Use new level of rotate logic @@ -181,70 +185,98 @@ func createRotateOrUpgradePlanInternal(log zerolog.Logger, apiObject k8sutil.API // podNeedsUpgrading decides if an upgrade of the pod is needed (to comply with // the given spec) and if that is allowed. -func podNeedsUpgrading(log zerolog.Logger, p *core.Pod, spec api.DeploymentSpec, images api.ImageInfoList) upgradeDecision { - if c, found := k8sutil.GetContainerByName(p, k8sutil.ServerContainerName); found { - specImageInfo, found := images.GetByImage(spec.GetImage()) - if !found { - return upgradeDecision{UpgradeNeeded: false} +func podNeedsUpgrading(log zerolog.Logger, status api.MemberStatus, spec api.DeploymentSpec, images api.ImageInfoList) upgradeDecision { + currentImage, found := currentImageInfo(spec, images) + if !found { + // Hold rotation tasks - we do not know image + return upgradeDecision{Hold: true} + } + + memberImage, found := memberImageInfo(spec, status, images) + if !found { + // Member info not found + return upgradeDecision{UpgradeNeeded: false} + } + + if currentImage.Image == memberImage.Image { + // No change + return upgradeDecision{UpgradeNeeded: false} + } + // Image changed, check if change is allowed + specVersion := currentImage.ArangoDBVersion + memberVersion := memberImage.ArangoDBVersion + asLicense := func(info api.ImageInfo) upgraderules.License { + if info.Enterprise { + return upgraderules.LicenseEnterprise } - podImageInfo, found := images.GetByImageID(c.Image) - if !found { - return upgradeDecision{UpgradeNeeded: false} - } - if specImageInfo.ImageID == podImageInfo.ImageID { - // No change - return upgradeDecision{UpgradeNeeded: false} - } - // Image changed, check if change is allowed - specVersion := specImageInfo.ArangoDBVersion - podVersion := podImageInfo.ArangoDBVersion - asLicense := func(info api.ImageInfo) upgraderules.License { - if info.Enterprise { - return upgraderules.LicenseEnterprise - } - return upgraderules.LicenseCommunity - } - specLicense := asLicense(specImageInfo) - podLicense := asLicense(podImageInfo) - if err := upgraderules.CheckUpgradeRulesWithLicense(podVersion, specVersion, podLicense, specLicense); err != nil { - // E.g. 3.x -> 4.x, we cannot allow automatically - return upgradeDecision{ - FromVersion: podVersion, - FromLicense: podLicense, - ToVersion: specVersion, - ToLicense: specLicense, - UpgradeNeeded: true, - UpgradeAllowed: false, - } - } - if specVersion.Major() != podVersion.Major() || specVersion.Minor() != podVersion.Minor() { - // Is allowed, with `--database.auto-upgrade` - log.Info().Str("spec-version", string(specVersion)).Str("pod-version", string(podVersion)). - Int("spec-version.major", specVersion.Major()).Int("spec-version.minor", specVersion.Minor()). - Int("pod-version.major", podVersion.Major()).Int("pod-version.minor", podVersion.Minor()). - Str("pod", p.GetName()).Msg("Deciding to do a upgrade with --auto-upgrade") - return upgradeDecision{ - FromVersion: podVersion, - FromLicense: podLicense, - ToVersion: specVersion, - ToLicense: specLicense, - UpgradeNeeded: true, - UpgradeAllowed: true, - AutoUpgradeNeeded: true, - } - } - // Patch version change, rotate only + return upgraderules.LicenseCommunity + } + specLicense := asLicense(currentImage) + memberLicense := asLicense(memberImage) + if err := upgraderules.CheckUpgradeRulesWithLicense(memberVersion, specVersion, memberLicense, specLicense); err != nil { + // E.g. 3.x -> 4.x, we cannot allow automatically return upgradeDecision{ - FromVersion: podVersion, - FromLicense: podLicense, + FromVersion: memberVersion, + FromLicense: memberLicense, + ToVersion: specVersion, + ToLicense: specLicense, + UpgradeNeeded: true, + UpgradeAllowed: false, + } + } + if specVersion.Major() != memberVersion.Major() || specVersion.Minor() != memberVersion.Minor() { + // Is allowed, with `--database.auto-upgrade` + log.Info().Str("spec-version", string(specVersion)).Str("pod-version", string(memberVersion)). + Int("spec-version.major", specVersion.Major()).Int("spec-version.minor", specVersion.Minor()). + Int("pod-version.major", memberVersion.Major()).Int("pod-version.minor", memberVersion.Minor()). + Msg("Deciding to do a upgrade with --auto-upgrade") + return upgradeDecision{ + FromVersion: memberVersion, + FromLicense: memberLicense, ToVersion: specVersion, ToLicense: specLicense, UpgradeNeeded: true, UpgradeAllowed: true, - AutoUpgradeNeeded: false, + AutoUpgradeNeeded: true, } } - return upgradeDecision{UpgradeNeeded: false} + // Patch version change, rotate only + return upgradeDecision{ + FromVersion: memberVersion, + FromLicense: memberLicense, + ToVersion: specVersion, + ToLicense: specLicense, + UpgradeNeeded: true, + UpgradeAllowed: true, + AutoUpgradeNeeded: false, + } +} + +func currentImageInfo(spec api.DeploymentSpec, images api.ImageInfoList) (api.ImageInfo, bool) { + if i, ok := images.GetByImage(spec.GetImage()); ok { + return i, true + } + if i, ok := images.GetByImageID(spec.GetImage()); ok { + return i, true + } + + return api.ImageInfo{}, false +} + +func memberImageInfo(spec api.DeploymentSpec, status api.MemberStatus, images api.ImageInfoList) (api.ImageInfo, bool) { + if status.Image != nil { + return *status.Image, true + } + + if i, ok := images.GetByImage(spec.GetImage()); ok { + return i, true + } + + if i, ok := images.GetByImageID(spec.GetImage()); ok { + return i, true + } + + return api.ImageInfo{}, false } // podNeedsRotation returns true when the specification of the @@ -305,7 +337,7 @@ func clusterReadyForUpgrade(context PlanBuilderContext) bool { // createUpgradeMemberPlan creates a plan to upgrade (stop-recreateWithAutoUpgrade-stop-start) an existing // member. func createUpgradeMemberPlan(log zerolog.Logger, member api.MemberStatus, - group api.ServerGroup, reason string, imageName string, status api.DeploymentStatus, rotateStatefull bool) api.Plan { + group api.ServerGroup, reason string, spec api.DeploymentSpec, status api.DeploymentStatus, rotateStatefull bool) api.Plan { upgradeAction := api.ActionTypeUpgradeMember if rotateStatefull || group.IsStateless() { upgradeAction = api.ActionTypeRotateMember @@ -317,14 +349,14 @@ func createUpgradeMemberPlan(log zerolog.Logger, member api.MemberStatus, Str("action", string(upgradeAction)). Msg("Creating upgrade plan") var plan api.Plan - if status.CurrentImage == nil || status.CurrentImage.Image != imageName { + if status.CurrentImage == nil || status.CurrentImage.Image != spec.GetImage() { plan = append(plan, - api.NewAction(api.ActionTypeSetCurrentImage, group, "", reason).SetImage(imageName), + api.NewAction(api.ActionTypeSetCurrentImage, group, "", reason).SetImage(spec.GetImage()), ) } - if member.Image == nil || member.Image.Image != imageName { + if member.Image == nil || member.Image.Image != spec.GetImage() { plan = append(plan, - api.NewAction(api.ActionTypeSetMemberCurrentImage, group, member.ID, reason).SetImage(imageName), + api.NewAction(api.ActionTypeSetMemberCurrentImage, group, member.ID, reason).SetImage(spec.GetImage()), ) } plan = append(plan, diff --git a/pkg/deployment/reconcile/plan_builder_rotate_upgrade_test.go b/pkg/deployment/reconcile/plan_builder_rotate_upgrade_test.go new file mode 100644 index 000000000..4ec738cab --- /dev/null +++ b/pkg/deployment/reconcile/plan_builder_rotate_upgrade_test.go @@ -0,0 +1,110 @@ +// +// DISCLAIMER +// +// Copyright 2020 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// +// Author Adam Janikowski +// + +package reconcile + +import ( + "testing" + + "github.com/arangodb/go-driver" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" + "github.com/arangodb/kube-arangodb/pkg/util" + "github.com/rs/zerolog/log" + "github.com/stretchr/testify/require" +) + +func Test_RotateUpgrade_Condition(t *testing.T) { + type testCase struct { + status api.MemberStatus + spec api.DeploymentSpec + images api.ImageInfoList + + verify func(t *testing.T, decision upgradeDecision) + } + + newImageInfo := func(image string, imageID string, version driver.Version, enterprise bool) api.ImageInfo { + return api.ImageInfo{ + Image: image, + ImageID: imageID, + ArangoDBVersion: version, + Enterprise: enterprise, + } + } + newImageInfoP := func(image string, imageID string, version driver.Version, enterprise bool) *api.ImageInfo { + p := newImageInfo(image, imageID, version, enterprise) + return &p + } + + newSpec := func(image string, mode api.DeploymentImageDiscoveryModeSpec) api.DeploymentSpec { + return api.DeploymentSpec{ + Image: util.NewString(image), + ImageDiscoveryMode: api.NewDeploymentImageDiscoveryModeSpec(mode), + } + } + + testCases := map[string]testCase{ + "Unknown spec image - wait for discovery": { + spec: newSpec("unknown", api.DeploymentImageDiscoveryKubeletMode), + images: api.ImageInfoList{}.Add(newImageInfo("a", "aid", "3.7.0", true)), + + verify: func(t *testing.T, decision upgradeDecision) { + require.True(t, decision.Hold) + }, + }, + "Missing image info": { + spec: newSpec("a", api.DeploymentImageDiscoveryKubeletMode), + images: api.ImageInfoList{}.Add(newImageInfo("a", "aid", "3.7.0", true)), + + verify: func(t *testing.T, decision upgradeDecision) { + require.False(t, decision.UpgradeNeeded) + }, + }, + "Upgrade Kubelet case": { + spec: newSpec("b", api.DeploymentImageDiscoveryKubeletMode), + status: api.MemberStatus{ + Image: newImageInfoP("a", "aid", "3.7.0", true), + }, + images: api.ImageInfoList{}.Add(newImageInfo("a", "aid", "3.7.0", true), newImageInfo("b", "bid", "3.8.0", true)), + + verify: func(t *testing.T, decision upgradeDecision) { + require.True(t, decision.UpgradeNeeded) + }, + }, + "Upgrade Direct case": { + spec: newSpec("b", api.DeploymentImageDiscoveryDirectMode), + status: api.MemberStatus{ + Image: newImageInfoP("a", "aid", "3.7.0", true), + }, + images: api.ImageInfoList{}.Add(newImageInfo("a", "aid", "3.7.0", true), newImageInfo("b", "bid", "3.8.0", true)), + + verify: func(t *testing.T, decision upgradeDecision) { + require.True(t, decision.UpgradeNeeded) + }, + }, + } + + for n, c := range testCases { + t.Run(n, func(t *testing.T) { + c.verify(t, podNeedsUpgrading(log.Logger, c.status, c.spec, c.images)) + }) + } +} diff --git a/pkg/deployment/resources/pod_creator.go b/pkg/deployment/resources/pod_creator.go index 052dcc038..3a54b588b 100644 --- a/pkg/deployment/resources/pod_creator.go +++ b/pkg/deployment/resources/pod_creator.go @@ -58,7 +58,12 @@ func versionHasAdvertisedEndpoint(v driver.Version) bool { } // createArangodArgs creates command line arguments for an arangod server in the given group. -func createArangodArgs(input pod.Input) []string { +func createArangodArgsWithUpgrade(input pod.Input, additionalOptions ...k8sutil.OptionPair) []string { + return createArangodArgs(input, pod.AutoUpgrade().Args(input)...) +} + +// createArangodArgs creates command line arguments for an arangod server in the given group. +func createArangodArgs(input pod.Input, additionalOptions ...k8sutil.OptionPair) []string { options := k8sutil.CreateOptionPairs(64) //scheme := NewURLSchemes(bsCfg.SslKeyFile != "").Arangod @@ -78,6 +83,8 @@ func createArangodArgs(input pod.Input) []string { // Logging options.Add("--log.level", "INFO") + options.Append(additionalOptions...) + // TLS options.Merge(pod.TLS().Args(input)) @@ -87,7 +94,6 @@ func createArangodArgs(input pod.Input) []string { options.Add("--database.directory", k8sutil.ArangodVolumeMountDir) options.Add("--log.output", "+") - options.Merge(pod.AutoUpgrade().Args(input)) options.Merge(pod.SNI().Args(input)) versionHasAdvertisedEndpoint := versionHasAdvertisedEndpoint(input.Version) @@ -299,7 +305,7 @@ func (r *Resources) RenderPodForMember(cachedStatus inspector.Inspector, spec ap // Render pod if group.IsArangod() { // Prepare arguments - autoUpgrade := m.Conditions.IsTrue(api.ConditionTypeAutoUpgrade) + autoUpgrade := m.Conditions.IsTrue(api.ConditionTypeAutoUpgrade) || spec.Upgrade.Get().AutoUpgrade memberPod := MemberArangoDPod{ status: m, diff --git a/pkg/deployment/resources/pod_creator_agent_args_test.go b/pkg/deployment/resources/pod_creator_agent_args_test.go index 4825bf26b..b3d974a62 100644 --- a/pkg/deployment/resources/pod_creator_agent_args_test.go +++ b/pkg/deployment/resources/pod_creator_agent_args_test.go @@ -119,7 +119,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { AutoUpgrade: true, ID: "a1", } - cmdline := createArangodArgs(input) + cmdline := createArangodArgsWithUpgrade(input) assert.Equal(t, []string{ "--agency.activate=true", diff --git a/pkg/deployment/resources/pod_creator_arangod.go b/pkg/deployment/resources/pod_creator_arangod.go index a46ef531e..2bc2826d9 100644 --- a/pkg/deployment/resources/pod_creator_arangod.go +++ b/pkg/deployment/resources/pod_creator_arangod.go @@ -419,11 +419,32 @@ func (m *MemberArangoDPod) GetInitContainers() ([]core.Container, error) { engine := m.spec.GetStorageEngine().AsArangoArgument() requireUUID := m.group == api.ServerGroupDBServers && m.status.IsInitialized - c := k8sutil.ArangodInitContainer("uuid", m.status.ID, engine, executable, operatorUUIDImage, requireUUID, + c := k8sutil.ArangodInitContainer(api.ServerGroupReservedInitContainerNameUUID, m.status.ID, engine, executable, operatorUUIDImage, requireUUID, m.groupSpec.SecurityContext.NewSecurityContext()) initContainers = append(initContainers, c) } + { + // Upgrade container - run in background + if m.autoUpgrade { + args := createArangodArgsWithUpgrade(m.AsInput()) + + c, err := k8sutil.NewContainer(args, m.GetContainerCreator()) + if err != nil { + return nil, err + } + + _, c.VolumeMounts = m.GetVolumes() + + c.Name = api.ServerGroupReservedInitContainerNameUpgrade + c.Lifecycle = nil + c.LivenessProbe = nil + c.ReadinessProbe = nil + + initContainers = append(initContainers, c) + } + } + return initContainers, nil } diff --git a/pkg/deployment/resources/pod_creator_coordinator_args_test.go b/pkg/deployment/resources/pod_creator_coordinator_args_test.go index 80112c9b2..0a4aeb1b2 100644 --- a/pkg/deployment/resources/pod_creator_coordinator_args_test.go +++ b/pkg/deployment/resources/pod_creator_coordinator_args_test.go @@ -116,7 +116,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { AutoUpgrade: true, ID: "id1", } - cmdline := createArangodArgs(input) + cmdline := createArangodArgsWithUpgrade(input) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", @@ -169,7 +169,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { AutoUpgrade: true, ID: "id1", } - cmdline := createArangodArgs(input) + cmdline := createArangodArgsWithUpgrade(input) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", diff --git a/pkg/deployment/resources/pod_creator_dbserver_args_test.go b/pkg/deployment/resources/pod_creator_dbserver_args_test.go index ca4e0532e..86b5fb7df 100644 --- a/pkg/deployment/resources/pod_creator_dbserver_args_test.go +++ b/pkg/deployment/resources/pod_creator_dbserver_args_test.go @@ -116,7 +116,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { AutoUpgrade: true, ID: "id1", } - cmdline := createArangodArgs(input) + cmdline := createArangodArgsWithUpgrade(input) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", diff --git a/pkg/deployment/resources/pod_creator_single_args_test.go b/pkg/deployment/resources/pod_creator_single_args_test.go index 8e3eb3bf2..7f357f8bb 100644 --- a/pkg/deployment/resources/pod_creator_single_args_test.go +++ b/pkg/deployment/resources/pod_creator_single_args_test.go @@ -91,7 +91,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { AutoUpgrade: true, ID: "a1", } - cmdline := createArangodArgs(input) + cmdline := createArangodArgsWithUpgrade(input) assert.Equal(t, []string{ "--database.auto-upgrade=true",