Skip to content

Commit

Permalink
fix(setting): move the filed Applied to Status
Browse files Browse the repository at this point in the history
Ref: longhorn/longhorn#8070

Signed-off-by: James Lu <james.lu@suse.com>
  • Loading branch information
mantissahz authored and innobead committed May 13, 2024
1 parent 79b09b9 commit 7fbf3fb
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 33 deletions.
6 changes: 0 additions & 6 deletions controller/instance_manager_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,12 +626,6 @@ func (imc *InstanceManagerController) areDangerZoneSettingsSyncedToIMPod(im *lon
if !isSettingSynced {
return false, false, false, nil
}
if !setting.Applied {
setting.Applied = true
if _, err := imc.ds.UpdateSetting(setting); err != nil {
imc.logger.WithError(err).Debugf("Failed to update setting %v applied", setting.Name)
}
}
}

return true, false, false, nil
Expand Down
22 changes: 12 additions & 10 deletions controller/setting_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,22 +219,29 @@ func (sc *SettingController) handleErr(err error, key interface{}) {
}

func (sc *SettingController) syncSetting(key string) (err error) {
defer func() {
err = errors.Wrapf(err, "failed to sync setting for %v", key)
}()

_, name, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
return err
}

defer func() {
err = errors.Wrapf(err, "failed to sync setting for %v", key)

setting, dsErr := sc.ds.GetSettingExact(types.SettingName(name))
if dsErr != nil {
sc.logger.WithError(dsErr).Warnf("Failed to get setting: %v", name)
return
}
if setting.Applied != (err == nil) {
setting.Applied = err == nil
if _, dsErr := sc.ds.UpdateSetting(setting); dsErr != nil {
existingApplied := setting.Status.Applied
if !setting.Status.Applied && err == nil {
setting.Status.Applied = true
} else if err != nil {
setting.Status.Applied = false
}
if setting.Status.Applied != existingApplied {
if _, dsErr := sc.ds.UpdateSettingStatus(setting); dsErr != nil {
sc.logger.WithError(dsErr).Warnf("Failed to update setting: %v", name)
}
}
Expand Down Expand Up @@ -1178,16 +1185,13 @@ func (sc *SettingController) syncUpgradeChecker() error {

if !upgradeCheckerEnabled {
if latestLonghornVersion.Value != "" {

latestLonghornVersion.Value = ""
latestLonghornVersion.Applied = false
if _, err := sc.ds.UpdateSetting(latestLonghornVersion); err != nil {
return err
}
}
if stableLonghornVersions.Value != "" {
stableLonghornVersions.Value = ""
stableLonghornVersions.Applied = false
if _, err := sc.ds.UpdateSetting(stableLonghornVersions); err != nil {
return err
}
Expand Down Expand Up @@ -1215,7 +1219,6 @@ func (sc *SettingController) syncUpgradeChecker() error {
sc.lastUpgradeCheckedTimestamp = now

if latestLonghornVersion.Value != currentLatestVersion {
stableLonghornVersions.Applied = true
sc.logger.Infof("Latest Longhorn version is %v", latestLonghornVersion.Value)
if _, err := sc.ds.UpdateSetting(latestLonghornVersion); err != nil {
// non-critical error, don't retry
Expand All @@ -1224,7 +1227,6 @@ func (sc *SettingController) syncUpgradeChecker() error {
}
}
if stableLonghornVersions.Value != currentStableVersions {
stableLonghornVersions.Applied = true
sc.logger.Infof("The latest stable version of every minor release line: %v", stableLonghornVersions.Value)
if _, err := sc.ds.UpdateSetting(stableLonghornVersions); err != nil {
// non-critical error, don't retry
Expand Down
1 change: 0 additions & 1 deletion controller/system_rollout_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1730,7 +1730,6 @@ func (c *SystemRolloutController) restoreSettings() (err error) {
if !ok {
return nil, fmt.Errorf(SystemRolloutErrFailedConvertToObjectFmt, exist.GetObjectKind(), types.LonghornKindSetting)
}
obj.Applied = true
return c.ds.UpdateSetting(obj)
}
_, err = c.rolloutResource(exist, fnUpdate, isSkipped, log, SystemRolloutMsgSkipIdentical)
Expand Down
13 changes: 12 additions & 1 deletion datastore/longhorn.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ func (s *DataStore) createOrUpdateSetting(name types.SettingName, value, default
}
setting.Annotations[types.GetLonghornLabelKey(types.ConfigMapResourceVersionKey)] = defaultSettingCMResourceVersion
setting.Value = value
setting.Applied = true

_, err = s.UpdateSetting(setting)
return err
Expand Down Expand Up @@ -260,6 +259,18 @@ func (s *DataStore) UpdateSetting(setting *longhorn.Setting) (*longhorn.Setting,
return obj, nil
}

// UpdateSettingStatus updates Longhorn Setting status and verifies update
func (s *DataStore) UpdateSettingStatus(setting *longhorn.Setting) (*longhorn.Setting, error) {
obj, err := s.lhClient.LonghornV1beta2().Settings(s.namespace).UpdateStatus(context.TODO(), setting, metav1.UpdateOptions{})
if err != nil {
return nil, err
}
verifyUpdate(setting.Name, obj, func(name string) (runtime.Object, error) {
return s.getSettingRO(name)
})
return obj, nil
}

// ValidateSetting checks the given setting value types and condition
func (s *DataStore) ValidateSetting(name, value string) (err error) {
defer func() {
Expand Down
15 changes: 10 additions & 5 deletions k8s/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2828,7 +2828,7 @@ spec:
name: Value
type: string
- description: The setting is applied
jsonPath: .applied
jsonPath: .status.applied
name: Applied
type: boolean
- jsonPath: .metadata.creationTimestamp
Expand All @@ -2842,19 +2842,24 @@ spec:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
applied:
description: The setting is applied.
type: boolean
kind:
description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
status:
description: SettingStatus defines the observed state of the Longhorn setting
properties:
applied:
description: The setting is applied.
type: boolean
required:
- applied
type: object
value:
description: The value of the setting.
type: string
required:
- applied
- value
type: object
served: true
Expand Down
9 changes: 8 additions & 1 deletion k8s/pkg/apis/longhorn/v1beta2/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
// +kubebuilder:subresource:status
// +kubebuilder:storageversion
// +kubebuilder:printcolumn:name="Value",type=string,JSONPath=`.value`,description="The value of the setting"
// +kubebuilder:printcolumn:name="Applied",type=boolean,JSONPath=`.applied`,description="The setting is applied"
// +kubebuilder:printcolumn:name="Applied",type=boolean,JSONPath=`.status.applied`,description="The setting is applied"
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`

// Setting is where Longhorn stores setting object.
Expand All @@ -18,6 +18,13 @@ type Setting struct {

// The value of the setting.
Value string `json:"value"`

// The status of the setting.
Status SettingStatus `json:"status,omitempty"`
}

// SettingStatus defines the observed state of the Longhorn setting
type SettingStatus struct {
// The setting is applied.
Applied bool `json:"applied"`
}
Expand Down
17 changes: 17 additions & 0 deletions k8s/pkg/apis/longhorn/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion manager/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func (m *VolumeManager) CreateOrUpdateSetting(s *longhorn.Setting) (*longhorn.Se
if err != nil {
return nil, err
}
s.Applied = true
setting, err := m.ds.UpdateSetting(s)
if err != nil {
if apierrors.IsNotFound(err) {
Expand Down
20 changes: 20 additions & 0 deletions upgrade/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,8 @@ func UpdateResourcesStatus(namespace string, lhClient *lhclientset.Clientset, re
err = updateEngineImageStatus(namespace, lhClient, resourceMap.(map[string]*longhorn.EngineImage))
case types.LonghornKindEngine:
err = updateEngineStatus(namespace, lhClient, resourceMap.(map[string]*longhorn.Engine))
case types.LonghornKindSetting:
err = updateSettingStatus(namespace, lhClient, resourceMap.(map[string]*longhorn.Setting))
default:
return fmt.Errorf("resource kind %v is not able to updated", resourceKind)
}
Expand Down Expand Up @@ -1167,3 +1169,21 @@ func updateEngineStatus(namespace string, lhClient *lhclientset.Clientset, engin
}
return nil
}

func updateSettingStatus(namespace string, lhClient *lhclientset.Clientset, settings map[string]*longhorn.Setting) error {
existingSettingList, err := lhClient.LonghornV1beta2().Settings(namespace).List(context.TODO(), metav1.ListOptions{})
if err != nil {
return err
}
for _, existingSetting := range existingSettingList.Items {
setting, ok := settings[existingSetting.Name]
if !ok {
continue
}

if _, err = lhClient.LonghornV1beta2().Settings(namespace).UpdateStatus(context.TODO(), setting, metav1.UpdateOptions{}); err != nil {
return err
}
}
return nil
}
16 changes: 8 additions & 8 deletions upgrade/v16xto170/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ func UpgradeResources(namespace string, lhClient *lhclientset.Clientset, kubeCli
return err
}

if err := upgradeNodes(namespace, lhClient, resourceMaps); err != nil {
return err
}

return upgradeSetting(namespace, lhClient, resourceMaps)
return upgradeNodes(namespace, lhClient, resourceMaps)
}

func UpgradeResourcesStatus(namespace string, lhClient *lhclientset.Clientset, kubeClient *clientset.Clientset, resourceMaps map[string]interface{}) error {
// We will probably need to upgrade other resource status as well. See upgradeEngineStatus or previous Longhorn
// versions for examples.
return upgradeEngineStatus(namespace, lhClient, resourceMaps)
if err := upgradeEngineStatus(namespace, lhClient, resourceMaps); err != nil {
return err
}

return upgradeSettingStatus(namespace, lhClient, resourceMaps)
}

func upgradeVolumes(namespace string, lhClient *lhclientset.Clientset, resourceMaps map[string]interface{}) (err error) {
Expand Down Expand Up @@ -90,7 +90,7 @@ func upgradeReplicas(namespace string, lhClient *lhclientset.Clientset, resource
return nil
}

func upgradeSetting(namespace string, lhClient *lhclientset.Clientset, resourceMaps map[string]interface{}) (err error) {
func upgradeSettingStatus(namespace string, lhClient *lhclientset.Clientset, resourceMaps map[string]interface{}) (err error) {
defer func() {
err = errors.Wrapf(err, upgradeLogPrefix+"upgrade setting failed")
}()
Expand All @@ -104,7 +104,7 @@ func upgradeSetting(namespace string, lhClient *lhclientset.Clientset, resourceM
}

for _, s := range settingMap {
s.Applied = true
s.Status.Applied = true
}

return nil
Expand Down

0 comments on commit 7fbf3fb

Please sign in to comment.