Skip to content

fix(upgrade): move osTreeRefresh after post-upgrade hooks to prevent rollback#442

Merged
electricface merged 1 commit into
linuxdeepin:masterfrom
electricface:swt/fix-bug-6-16
Jun 16, 2026
Merged

fix(upgrade): move osTreeRefresh after post-upgrade hooks to prevent rollback#442
electricface merged 1 commit into
linuxdeepin:masterfrom
electricface:swt/fix-bug-6-16

Conversation

@electricface

Copy link
Copy Markdown
Member

Second reboot after upgrade triggered automatic rollback because
/etc/deepin/deepin_update_option.json was missing in the new deployment.

Root cause: when osTreeRefresh() is called with fullMerge=true, it creates a
fresh deployment that discards all /etc modifications. Since osTreeRefresh()
was called before preUpgradeCmdSuccessHook(), which writes
/etc/deepin/deepin_update_option.json, that file was absent in the new
deployment, causing the system to treat the upgrade as failed and roll back.
When fullMerge=false, /etc modifications are preserved in the new deployment,
so the bug does not occur in that case.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @electricface, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

…rollback

Second reboot after upgrade triggered automatic rollback because
/etc/deepin/deepin_update_option.json was missing in the new deployment.

Root cause: when osTreeRefresh() is called with fullMerge=true, it creates a
fresh deployment that discards all /etc modifications. Since osTreeRefresh()
was called before preUpgradeCmdSuccessHook(), which writes
/etc/deepin/deepin_update_option.json, that file was absent in the new
deployment, causing the system to treat the upgrade as failed and roll back.
When fullMerge=false, /etc modifications are preserved in the new deployment,
so the bug does not occur in that case.

Log: 修复全量合并升级后第二次重启自动回滚的问题
PMS: TASK-391183
Influence: 系统升级
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@electricface electricface merged commit 0ccf45b into linuxdeepin:master Jun 16, 2026
13 of 18 checks passed
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码正确调整了不可变系统刷新时机并清理了废弃参数,逻辑清晰且无副作用
语法、质量、性能均无问题且无安全漏洞,满分通过

■ 【详细分析】

  • 1.语法逻辑(正确)✓

distUpgrade 函数中移除了提前执行 osTreeRefresh 的代码块,并将其移动到 preUpgradeCmdSuccessHook 函数内部末尾。同时准确地将函数签名中的废弃参数 needChangeGrub bool 替换为 refreshFullMerge bool,调用处的实参和形参顺序、数量、类型完全匹配,控制流未受破坏
建议:无需改进

  • 2.代码质量(良好)✓

移除了无用的 needChangeGrub 参数,消除了代码坏味道,提升了函数接口的清晰度。将 osTreeRefresh 调用后移至升级流程末尾,配合新增的注释 Perform immutable system refresh at the end of the upgrade process,使代码意图更加明确,符合单一职责原则
建议:无需改进

  • 3.代码性能(无性能问题)✓

代码修改仅涉及逻辑顺序的调整和参数传递的变更,osTreeRefresh 的调用次数从一次变为一次,未引入额外的系统调用、循环或内存分配,对整体执行开销无影响
建议:无需改进

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
未发现安全风险,本次变更仅涉及内部状态管理和布尔参数传递,不存在外部输入校验缺失、命令注入或权限绕过等攻击面

  • 建议:保持现有的安全编码规范

■ 【改进建议代码示例】

// 当前代码已足够优秀,无需额外修改,此处展示当前最终态的合规代码片段供参考
func (m *Manager) preUpgradeCmdSuccessHook(job *Job, mode system.UpdateType, uuid string, refreshFullMerge bool) error {
	if !m.config.GetPlatformStatusDisable(config.DisabledRebootCheck) {
		err := m.setRebootCheckOption(mode, uuid)
		if err != nil {
			return err
		}
	} else {
		m.handleAfterUpgradeSuccess(mode, job.Description, uuid)
	}
	// Perform immutable system refresh at the end of the upgrade process
	if m.statusManager.abStatus == system.HasBackedUp {
		if err := m.immutableManager.osTreeRefresh(refreshFullMerge); err != nil {
			logger.Warning("ostree deploy refresh failed,", err)
		}
	}

	m.statusManager.SetUpdateStatus(mode, system.Upgraded)
	job.setPropProgress(1.00)
	return nil
}

@electricface electricface deleted the swt/fix-bug-6-16 branch June 16, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants