Skip to content

Commit

Permalink
deploy_ctrl: fix active configmap not work, fix updated status (#233)
Browse files Browse the repository at this point in the history
Signed-off-by: haorenfsa <[email protected]>
  • Loading branch information
haorenfsa authored Jan 14, 2025
1 parent 90ff623 commit a7cf9b1
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 50 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
IMG ?= milvusdb/milvus-operator:dev-latest
TOOL_IMG ?= milvus-config-tool:dev-latest
SIT_IMG ?= milvus-operator:sit
VERSION ?= 1.1.7
VERSION ?= 1.1.8
TOOL_VERSION ?= 1.0.0
MILVUS_HELM_VERSION ?= milvus-4.2.13
RELEASE_IMG ?= milvusdb/milvus-operator:v$(VERSION)
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ helm -n milvus-operator upgrade --install --create-namespace milvus-operator mil
Or with kubectl & raw manifests:

```shell
kubectl apply -f https://raw.githubusercontent.com/zilliztech/milvus-operator/v1.1.7/deploy/manifests/deployment.yaml
kubectl apply -f https://raw.githubusercontent.com/zilliztech/milvus-operator/v1.1.8/deploy/manifests/deployment.yaml
```

For more infomation Check [Installation Instructions](docs/installation/installation.md)
Expand Down Expand Up @@ -135,11 +135,11 @@ Use helm:
```shell
helm upgrade --install milvus-operator \
-n milvus-operator --create-namespace \
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.7/milvus-operator-1.1.7.tgz
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.8/milvus-operator-1.1.8.tgz
```

Or use kubectl & raw manifests:

```shell
kubectl apply -f https://raw.githubusercontent.com/zilliztech/milvus-operator/v1.1.7/deploy/manifests/deployment.yaml
kubectl apply -f https://raw.githubusercontent.com/zilliztech/milvus-operator/v1.1.8/deploy/manifests/deployment.yaml
```
4 changes: 2 additions & 2 deletions charts/milvus-operator/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 1.1.7
version: 1.1.8

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.
# It is recommended to use it with quotes.
appVersion: "1.1.7"
appVersion: "1.1.8"

dependencies:
- name: cert-manager
Expand Down
2 changes: 1 addition & 1 deletion charts/milvus-operator/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ image:
# image.pullPolicy -- The image pull policy for the controller.
pullPolicy: IfNotPresent
# image.tag -- The image tag whose default is the chart appVersion.
tag: "v1.1.7"
tag: "v1.1.8"

# installCRDs -- If true, CRD resources will be installed as part of the Helm chart. If enabled, when uninstalling CRD resources will be deleted causing all installed custom resources to be DELETED
installCRDs: true
Expand Down
18 changes: 9 additions & 9 deletions deploy/manifests/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ metadata:
name: "milvus-operator"
namespace: "milvus-operator"
labels:
helm.sh/chart: milvus-operator-1.1.7
helm.sh/chart: milvus-operator-1.1.8
app.kubernetes.io/name: milvus-operator
app.kubernetes.io/instance: milvus-operator
app.kubernetes.io/version: "1.1.7"
app.kubernetes.io/version: "1.1.8"
app.kubernetes.io/managed-by: Helm
---
# Source: milvus-operator/templates/crds.yaml
Expand Down Expand Up @@ -15933,10 +15933,10 @@ kind: Service
metadata:
labels:
service-kind: metrics
helm.sh/chart: milvus-operator-1.1.7
helm.sh/chart: milvus-operator-1.1.8
app.kubernetes.io/name: milvus-operator
app.kubernetes.io/instance: milvus-operator
app.kubernetes.io/version: "1.1.7"
app.kubernetes.io/version: "1.1.8"
app.kubernetes.io/managed-by: Helm
name: 'milvus-operator-metrics-service'
namespace: "milvus-operator"
Expand All @@ -15954,10 +15954,10 @@ apiVersion: v1
kind: Service
metadata:
labels:
helm.sh/chart: milvus-operator-1.1.7
helm.sh/chart: milvus-operator-1.1.8
app.kubernetes.io/name: milvus-operator
app.kubernetes.io/instance: milvus-operator
app.kubernetes.io/version: "1.1.7"
app.kubernetes.io/version: "1.1.8"
app.kubernetes.io/managed-by: Helm
name: 'milvus-operator-webhook-service'
namespace: "milvus-operator"
Expand All @@ -15976,10 +15976,10 @@ apiVersion: apps/v1
kind: Deployment
metadata:
labels:
helm.sh/chart: milvus-operator-1.1.7
helm.sh/chart: milvus-operator-1.1.8
app.kubernetes.io/name: milvus-operator
app.kubernetes.io/instance: milvus-operator
app.kubernetes.io/version: "1.1.7"
app.kubernetes.io/version: "1.1.8"
app.kubernetes.io/managed-by: Helm
name: "milvus-operator"
namespace: "milvus-operator"
Expand Down Expand Up @@ -16009,7 +16009,7 @@ spec:
- --leader-elect
command:
- /manager
image: 'milvusdb/milvus-operator:v1.1.7'
image: 'milvusdb/milvus-operator:v1.1.8'
imagePullPolicy: "IfNotPresent"
livenessProbe:
httpGet:
Expand Down
10 changes: 5 additions & 5 deletions docs/installation/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ For quick start, install with one line command:
```shell
helm install milvus-operator \
-n milvus-operator --create-namespace \
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.7/milvus-operator-1.1.7.tgz
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.8/milvus-operator-1.1.8.tgz
```

If you already have `cert-manager` v1.0+ installed which is not in its default configuration, you may encounter some error with the check of cert-manager installation. you can install with special options to disable the check:

```
helm install milvus-operator \
-n milvus-operator --create-namespace \
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.7/milvus-operator-1.1.7.tgz \
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.8/milvus-operator-1.1.8.tgz \
--set checker.disableCertManagerCheck=true
```

Expand All @@ -39,7 +39,7 @@ use helm commands to upgrade earlier milvus-operator to current version:

```shell
helm upgrade -n milvus-operator milvus-operator --reuse-values \
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.7/milvus-operator-1.1.7.tgz
https://github.com/zilliztech/milvus-operator/releases/download/v1.1.8/milvus-operator-1.1.8.tgz
```

## Delete operator
Expand All @@ -62,7 +62,7 @@ If you don't want to use helm you can also install with kubectl and raw manifest
## Installation
It is recommended to install the milvus operator with a newest stable version
```shell
kubectl apply -f https://github.com/zilliztech/milvus-operator/v1.1.7/deploy/manifests/deployment.yaml
kubectl apply -f https://github.com/zilliztech/milvus-operator/v1.1.8/deploy/manifests/deployment.yaml
```

Check the installed operators:
Expand All @@ -85,7 +85,7 @@ Same as installation, you can update the milvus operator with a newer version by
Delete the milvus operator stack by the deployment manifest:

```shell
kubectl delete -f https://github.com/zilliztech/milvus-operator/v1.1.7/deploy/manifests/deployment.yaml
kubectl delete -f https://github.com/zilliztech/milvus-operator/v1.1.8/deploy/manifests/deployment.yaml
```

Or delete the milvus operator stack by using makefile:
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/deploy_ctrl.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func (c *DeployControllerBizImpl) IsUpdating(ctx context.Context, mc v1beta1.Mil
if err != nil {
return false, errors.Wrap(err, "get querynode deployments")
}
newPodtemplate := c.util.RenderPodTemplateWithoutGroupID(mc, &deploy.Spec.Template, c.component)
newPodtemplate := c.util.RenderPodTemplateWithoutGroupID(mc, &deploy.Spec.Template, c.component, false)
return c.util.IsNewRollout(ctx, deploy, newPodtemplate), nil

}
Expand Down Expand Up @@ -290,7 +290,7 @@ func (c *DeployControllerBizImpl) HandleRolling(ctx context.Context, mc v1beta1.
if currentDeploy == nil {
return errors.Errorf("[%s]'s current deployment not found", c.component.Name)
}
podTemplate := c.util.RenderPodTemplateWithoutGroupID(mc, &currentDeploy.Spec.Template, c.component)
podTemplate := c.util.RenderPodTemplateWithoutGroupID(mc, &currentDeploy.Spec.Template, c.component, false)

if c.util.ShouldRollback(ctx, currentDeploy, lastDeploy, podTemplate) {
currentDeploy = lastDeploy
Expand Down Expand Up @@ -325,7 +325,7 @@ func (c *DeployControllerBizImpl) HandleManualMode(ctx context.Context, mc v1bet
if currentDeploy == nil {
return errors.Errorf("[%s]'s current deployment not found", c.component.Name)
}
podTemplate := c.util.RenderPodTemplateWithoutGroupID(mc, &currentDeploy.Spec.Template, c.component)
podTemplate := c.util.RenderPodTemplateWithoutGroupID(mc, &currentDeploy.Spec.Template, c.component, true)
if c.util.IsNewRollout(ctx, currentDeploy, podTemplate) {
return c.util.PrepareNewRollout(ctx, mc, currentDeploy, podTemplate)
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/controllers/deploy_ctrl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func TestDeployControllerBizImpl_IsUpdating(t *testing.T) {
deploy := appsv1.Deployment{}

mockUtil.EXPECT().GetOldDeploy(ctx, mc, component).Return(&deploy, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().IsNewRollout(ctx, &deploy, gomock.Any()).Return(true)
ret, err := bizImpl.IsUpdating(ctx, mc)

Expand All @@ -303,7 +303,7 @@ func TestDeployControllerBizImpl_IsUpdating(t *testing.T) {
deploy := appsv1.Deployment{}

mockUtil.EXPECT().GetOldDeploy(ctx, mc, component).Return(&deploy, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().IsNewRollout(ctx, &deploy, gomock.Any()).Return(false)
ret, err := bizImpl.IsUpdating(ctx, mc)

Expand Down Expand Up @@ -494,7 +494,7 @@ func TestDeployControllerBizImpl_HandleRolling(t *testing.T) {

t.Run("no rolling ok", func(t *testing.T) {
mockUtil.EXPECT().GetDeploys(ctx, mc).Return(&deploy, nil, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().ShouldRollback(ctx, &deploy, nil, nil).Return(false)
mockUtil.EXPECT().LastRolloutFinished(ctx, mc, &deploy, nil).Return(true, nil)
mockUtil.EXPECT().IsNewRollout(ctx, &deploy, nil).Return(false)
Expand All @@ -504,7 +504,7 @@ func TestDeployControllerBizImpl_HandleRolling(t *testing.T) {

t.Run("roll back & requeue", func(t *testing.T) {
mockUtil.EXPECT().GetDeploys(ctx, mc).Return(&deploy, nil, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().ShouldRollback(ctx, &deploy, nil, nil).Return(true)
mockUtil.EXPECT().PrepareNewRollout(ctx, mc, nil, nil).Return(ErrRequeue)
err := bizImpl.HandleRolling(ctx, mc)
Expand All @@ -514,7 +514,7 @@ func TestDeployControllerBizImpl_HandleRolling(t *testing.T) {

t.Run("check last rollout failed", func(t *testing.T) {
mockUtil.EXPECT().GetDeploys(ctx, mc).Return(&deploy, nil, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().ShouldRollback(ctx, &deploy, nil, nil).Return(false)
mockUtil.EXPECT().LastRolloutFinished(ctx, mc, &deploy, nil).Return(false, errMock)
err := bizImpl.HandleRolling(ctx, mc)
Expand All @@ -523,7 +523,7 @@ func TestDeployControllerBizImpl_HandleRolling(t *testing.T) {

t.Run("continue last rollout not finished, ok", func(t *testing.T) {
mockUtil.EXPECT().GetDeploys(ctx, mc).Return(&deploy, nil, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().ShouldRollback(ctx, &deploy, nil, nil).Return(false)
mockUtil.EXPECT().LastRolloutFinished(ctx, mc, &deploy, nil).Return(false, nil)
err := bizImpl.HandleRolling(ctx, mc)
Expand All @@ -532,7 +532,7 @@ func TestDeployControllerBizImpl_HandleRolling(t *testing.T) {

t.Run("new rollout & requeue", func(t *testing.T) {
mockUtil.EXPECT().GetDeploys(ctx, mc).Return(&deploy, &deploy2, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, false).Return(nil)
mockUtil.EXPECT().ShouldRollback(ctx, &deploy, &deploy2, nil).Return(false)
mockUtil.EXPECT().LastRolloutFinished(ctx, mc, &deploy, &deploy2).Return(true, nil)
mockUtil.EXPECT().IsNewRollout(ctx, &deploy, nil).Return(true)
Expand Down Expand Up @@ -569,7 +569,7 @@ func TestDeployControllerBizImpl_HandleManualMode(t *testing.T) {
deploy := &appsv1.Deployment{}
t.Run("no rolling, renew deploy annotation, update requeue", func(t *testing.T) {
mockUtil.EXPECT().GetDeploys(ctx, mc).Return(deploy, nil, nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode).Return(nil)
mockUtil.EXPECT().RenderPodTemplateWithoutGroupID(mc, gomock.Any(), QueryNode, true).Return(nil)
mockUtil.EXPECT().IsNewRollout(ctx, deploy, nil).Return(false)
mockUtil.EXPECT().RenewDeployAnnotation(ctx, mc, deploy).Return(true)
mockUtil.EXPECT().UpdateAndRequeue(ctx, deploy).Return(ErrRequeue)
Expand Down
16 changes: 9 additions & 7 deletions pkg/controllers/deploy_ctrl_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

// DeployControllerBizUtil are the business logics of DeployControllerBizImpl, abstracted for unit test
type DeployControllerBizUtil interface {
RenderPodTemplateWithoutGroupID(mc v1beta1.Milvus, currentTemplate *corev1.PodTemplateSpec, component MilvusComponent) *corev1.PodTemplateSpec
RenderPodTemplateWithoutGroupID(mc v1beta1.Milvus, currentTemplate *corev1.PodTemplateSpec, component MilvusComponent, forceUpdateAll bool) *corev1.PodTemplateSpec

// GetDeploys returns currentDeployment, lastDeployment when there is exactly one currentDeployment, one lastDeployment
// otherwise return err. in particular:
Expand Down Expand Up @@ -88,17 +88,19 @@ func NewDeployControllerBizUtil(component MilvusComponent, cli client.Client, k8
}
}

func (c *DeployControllerBizUtilImpl) RenderPodTemplateWithoutGroupID(mc v1beta1.Milvus, currentTemplate *corev1.PodTemplateSpec, component MilvusComponent) *corev1.PodTemplateSpec {
func (c *DeployControllerBizUtilImpl) RenderPodTemplateWithoutGroupID(mc v1beta1.Milvus, currentTemplate *corev1.PodTemplateSpec, component MilvusComponent, forceUpdateAll bool) *corev1.PodTemplateSpec {
ret := new(corev1.PodTemplateSpec)
if currentTemplate != nil {
ret = currentTemplate.DeepCopy()
}
updater := newMilvusDeploymentUpdater(mc, c.cli.Scheme(), component)
appLabels := NewComponentAppLabels(updater.GetIntanceName(), updater.GetComponent().Name)
isCreating := currentTemplate == nil
isStopped := ReplicasValue(component.GetReplicas(mc.Spec)) == 0
updateDefaults := isCreating || isStopped
updatePodTemplate(updater, ret, appLabels, updateDefaults)
if !forceUpdateAll {
isCreating := currentTemplate == nil
isStopped := ReplicasValue(component.GetReplicas(mc.Spec)) == 0
forceUpdateAll = isCreating || isStopped
}
updatePodTemplate(updater, ret, appLabels, forceUpdateAll)
return ret
}

Expand Down Expand Up @@ -164,7 +166,7 @@ func formatComponentDeployName(mc v1beta1.Milvus, component MilvusComponent, gro

func (c *DeployControllerBizUtilImpl) CreateDeploy(ctx context.Context, mc v1beta1.Milvus, podTemplate *corev1.PodTemplateSpec, groupId int) error {
if podTemplate == nil {
podTemplate = c.RenderPodTemplateWithoutGroupID(mc, nil, c.component)
podTemplate = c.RenderPodTemplateWithoutGroupID(mc, nil, c.component, true)
}
if groupId != 0 {
// is not the first deploy, set image to dummy to avoid rolling back and forth
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/deploy_ctrl_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestDeployControllerBizUtilImpl_RenderPodTemplateWithoutGroupID(t *testing.
component := DataNode

mockcli.EXPECT().Scheme().Return(scheme)
template := bizUtil.RenderPodTemplateWithoutGroupID(mc, currentTemplate, component)
template := bizUtil.RenderPodTemplateWithoutGroupID(mc, currentTemplate, component, false)
assert.NotNil(t, template)
assert.Equal(t, template.Labels[v1beta1.GetComponentGroupIdLabel(component.Name)], "")
}
Expand Down Expand Up @@ -300,13 +300,13 @@ func TestDeployControllerBizUtilImpl_ShouldRollback(t *testing.T) {
currentDeploy := new(appsv1.Deployment)
lastDeploy := new(appsv1.Deployment)
mockcli.EXPECT().Scheme().Return(scheme).AnyTimes()
podTemplate := bizUtil.RenderPodTemplateWithoutGroupID(mc, nil, DataNode)
podTemplate := bizUtil.RenderPodTemplateWithoutGroupID(mc, nil, DataNode, false)
labelHelper := v1beta1.Labels()

t.Cleanup(func() {
currentDeploy = new(appsv1.Deployment)
lastDeploy = new(appsv1.Deployment)
podTemplate = bizUtil.RenderPodTemplateWithoutGroupID(mc, nil, DataNode)
podTemplate = bizUtil.RenderPodTemplateWithoutGroupID(mc, nil, DataNode, false)
mockCtrl.Finish()
})

Expand Down
14 changes: 7 additions & 7 deletions pkg/controllers/deployment_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ func updateDeployment(deployment *appsv1.Deployment, updater deploymentUpdater)
return err
}
isStopped := getDeployReplicas(deployment) == 0
updateDefaults := isCreating || isStopped
updatePodTemplate(updater, &deployment.Spec.Template, appLabels, updateDefaults)
forceUpdateAll := isCreating || isStopped
updatePodTemplate(updater, &deployment.Spec.Template, appLabels, forceUpdateAll)
return nil
}

Expand All @@ -88,15 +88,15 @@ func updatePodTemplate(
updater deploymentUpdater,
template *corev1.PodTemplateSpec,
appLabels map[string]string,
updateDefaults bool,
forceUpdateAll bool,
) {
currentTemplate := template.DeepCopy()

updatePodMeta(template, appLabels, updater)
updateInitContainers(template, updater)
updateUserDefinedVolumes(template, updater)
updateScheduleSpec(template, updater)
updateMilvusContainer(template, updater, updateDefaults)
updateMilvusContainer(template, updater, forceUpdateAll)
updateSidecars(template, updater)
updateNetworkSettings(template, updater)

Expand All @@ -114,7 +114,7 @@ func updatePodTemplate(
"namespace", updater.GetMilvus().Namespace,
"milvus", updater.GetMilvus().Name).
Info("pod template updated by crd", "diff", diff.ObjectDiff(currentTemplate, template))
case updateDefaults:
case forceUpdateAll:
default:
// no updates, no default changes
return
Expand Down Expand Up @@ -230,7 +230,7 @@ func updateBuiltInVolumes(template *corev1.PodTemplateSpec, updater deploymentUp
}
}

func updateMilvusContainer(template *corev1.PodTemplateSpec, updater deploymentUpdater, isCreating bool) {
func updateMilvusContainer(template *corev1.PodTemplateSpec, updater deploymentUpdater, forceUpdateImage bool) {
mergedComSpec := updater.GetMergedComponentSpec()

containerIdx := GetContainerIndex(template.Spec.Containers, updater.GetComponent().Name)
Expand Down Expand Up @@ -286,7 +286,7 @@ func updateMilvusContainer(template *corev1.PodTemplateSpec, updater deploymentU
}

container.ImagePullPolicy = *mergedComSpec.ImagePullPolicy
if isCreating ||
if forceUpdateImage ||
!updater.GetMilvus().IsRollingUpdateEnabled() || // rolling update is disabled
updater.GetMilvus().Spec.Com.ImageUpdateMode == v1beta1.ImageUpdateModeAll || // image update mode is update all
updater.GetMilvus().Spec.Com.ImageUpdateMode == v1beta1.ImageUpdateModeForce ||
Expand Down
Loading

0 comments on commit a7cf9b1

Please sign in to comment.