Skip to content

feat: Complete the node initialization logic #7571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
f2c-ci-robot merged 2 commits into from
Dec 26, 2024
Merged

feat: Complete the node initialization logic #7571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5398819
feat: Complete the node initialization logic
ssongliu Dec 26, 2024
cb3fd49
feat: Update backup api
ssongliu Dec 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions agent/app/model/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ type Setting struct {
}

type NodeInfo struct {
Scope string `json:"scope"`
BaseDir string `json:"baseDir"`
Version string `json:"version"`
EncryptKey string `json:"encryptKey"`
ServerCrt string `json:"serverCrt"`
ServerKey string `json:"serverKey"`
Scope string `json:"scope"`
BaseDir string `json:"baseDir"`
Version string `json:"version"`
ServerCrt string `json:"serverCrt"`
ServerKey string `json:"serverKey"`
}
16 changes: 16 additions & 0 deletions agent/app/repo/app_launcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ type ILauncherRepo interface {
Create(launcher *model.AppLauncher) error
Save(launcher *model.AppLauncher) error
Delete(opts ...DBOption) error

SyncAll(data []model.AppLauncher) error
}

func NewILauncherRepo() ILauncherRepo {
Expand Down Expand Up @@ -53,3 +55,17 @@ func (u *LauncherRepo) Delete(opts ...DBOption) error {
}
return db.Delete(&model.AppLauncher{}).Error
}

func (u *LauncherRepo) SyncAll(data []model.AppLauncher) error {
tx := global.DB.Begin()
if err := tx.Where("1 = 1").Delete(&model.AppLauncher{}).Error; err != nil {
tx.Rollback()
return err
}
if err := tx.Model(model.AppLauncher{}).Save(&data).Error; err != nil {
tx.Rollback()
return err
}
tx.Commit()
return nil
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 26, 2024

Choose a reason for hiding this comment

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

The most notable difference I found is a small typo in the code:

syncAll: `syncAll`

This should be corrected to ensure correct function names.

Regarding optimization suggestions or potential issues:

  • It's good practice to handle database operations carefully, especially when using BeginTransaction, Rollback, etc, because it can impact performance due to locking conflicts or resource contention.
  • In your case:
    • Ensure that you're checking if data exists before saving, rather than deleting all instances of launcher. Saving could cause some data to go into transient state if not done correctly.
    • Consider testing on different use cases to avoid hardcoding values which might break things under different scenarios

If you want more detailed suggestions, please include additional context about how this repo functions in the application pipeline, or what other features it has/dependencies.

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 26, 2024

Choose a reason for hiding this comment

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

This is a Go code snippet for an interface-based interface ILauncherRepo which extends the functionality of saving and deleting app launcher models to another interface named ISaveOrUpdate with various methods such as creating, updating, and deletion through database operations. Here are some observations:

  1. The commented-out line at SyncAll: There seems like this function could also be implemented using transactions (tx, Begin(), Rollback()).

There are no known errors or inconsistencies based on context provided.

No optimization suggestions needed here considering it appears not used directly in the actual application logic but serves an informational purpose.

16 changes: 16 additions & 0 deletions agent/app/repo/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ type IBackupRepo interface {
WithByFileName(fileName string) DBOption
WithByCronID(cronjobID uint) DBOption
WithFileNameStartWith(filePrefix string) DBOption

SyncAll(data []model.BackupAccount) error
}

func NewIBackupRepo() IBackupRepo {
Expand Down Expand Up @@ -153,3 +155,17 @@ func (u *BackupRepo) GetRecord(opts ...DBOption) (*model.BackupRecord, error) {
err := db.Find(record).Error
return record, err
}

func (u *BackupRepo) SyncAll(data []model.BackupAccount) error {
tx := global.DB.Begin()
if err := tx.Where("1 = 1").Delete(&model.BackupAccount{}).Error; err != nil {
tx.Rollback()
return err
}
if err := tx.Model(model.BackupAccount{}).Save(&data).Error; err != nil {
tx.Rollback()
return err
}
tx.Commit()
return nil
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 26, 2024

Choose a reason for hiding this comment

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

The code seems to be correctly formatted in both Go version and Java version. There doesn't appear to be any obvious errors or oddities that need adjustments.

One minor suggestion is for the function SyncAll in the Golang implementation to properly handle returning errors if the operation fails (e.g., using return, like in Python), which can provide more clear and concise feedback about successful operations compared to just returning the status of whether an "error" was encountered.

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 26, 2024

Choose a reason for hiding this comment

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

I'm sorry but the text you've provided is too short to be analyzed for specific code differences, issues or optimizations. For detailed examination of an entire file content it would be helpful if you specify what aspects exactly need examining like syntax mistakes, missing types etc.

For example, some key checks could include:

  • Syntax errors,
  • Missing function or method signatures or definitions,
  • Incomplete struct definition/field declaration,

etc.

1 change: 0 additions & 1 deletion agent/app/service/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ func (u *SettingService) ReloadConn() error {

global.CONF.System.BaseDir = nodeInfo.BaseDir
global.CONF.System.Version = nodeInfo.Version
global.CONF.System.EncryptKey = nodeInfo.EncryptKey
global.IsMaster = nodeInfo.Scope == "master"
return nil
}
2 changes: 1 addition & 1 deletion agent/init/hook/hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func initGlobalData() {
if err := settingRepo.Update("SystemStatus", "Free"); err != nil {
global.LOG.Fatalf("init service before start failed, err: %v", err)
}

global.CONF.System.EncryptKey, _ = settingRepo.GetValueByKey("EncryptKey")
_ = service.NewISettingService().ReloadConn()
if global.IsMaster {
global.CoreDB = common.LoadDBConnByPath(path.Join(global.CONF.System.DbPath, "core.db"), "core")
Expand Down
8 changes: 4 additions & 4 deletions agent/init/migration/migrations/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

var AddTable = &gormigrate.Migration{
ID: "20241009-add-table",
ID: "20241226-add-table",
Migrate: func(tx *gorm.DB) error {
return tx.AutoMigrate(
&model.AppDetail{},
Expand Down Expand Up @@ -81,9 +81,6 @@ var InitSetting = &gormigrate.Migration{
if err := tx.Create(&model.Setting{Key: "BaseDir", Value: nodeInfo.BaseDir}).Error; err != nil {
return err
}
if err := tx.Create(&model.Setting{Key: "EncryptKey", Value: nodeInfo.EncryptKey}).Error; err != nil {
return err
}
itemKey, _ := encrypt.StringEncrypt(nodeInfo.ServerKey)
if err := tx.Create(&model.Setting{Key: "ServerKey", Value: itemKey}).Error; err != nil {
return err
Expand All @@ -99,6 +96,9 @@ var InitSetting = &gormigrate.Migration{
return err
}

if err := tx.Create(&model.Setting{Key: "EncryptKey", Value: common.RandStr(16)}).Error; err != nil {
return err
}
if err := tx.Create(&model.Setting{Key: "SystemIP", Value: ""}).Error; err != nil {
return err
}
Expand Down
2 changes: 0 additions & 2 deletions agent/utils/xpack/xpack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/1Panel-dev/1Panel/agent/buserr"
"github.com/1Panel-dev/1Panel/agent/constant"
"github.com/1Panel-dev/1Panel/agent/utils/cmd"
"github.com/1Panel-dev/1Panel/agent/utils/common"
)

func RemoveTamper(website string) {}
Expand Down Expand Up @@ -48,7 +47,6 @@ func LoadNodeInfo() (bool, model.NodeInfo, error) {
var info model.NodeInfo
info.BaseDir = loadParams("BASE_DIR")
info.Version = loadParams("ORIGINAL_VERSION")
info.EncryptKey = common.RandStr(16)
info.Scope = "master"
return false, info, nil
}
Expand Down
15 changes: 8 additions & 7 deletions core/app/service/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,14 @@ func (u *BackupService) GetLocalDir() (string, error) {
}

func (u *BackupService) SearchWithPage(req dto.SearchPageWithType) (int64, interface{}, error) {
count, accounts, err := backupRepo.Page(
req.Page,
req.PageSize,
repo.WithByType(req.Type),
repo.WithByName(req.Info),
repo.WithOrderBy("created_at desc"),
)
options := []global.DBOption{repo.WithOrderBy("created_at desc")}
if len(req.Type) != 0 {
options = append(options, repo.WithByType(req.Type))
}
if len(req.Info) != 0 {
options = append(options, repo.WithByType(req.Info))
}
count, accounts, err := backupRepo.Page(req.Page, req.PageSize, options...)
if err != nil {
return 0, nil, err
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 26, 2024

Choose a reason for hiding this comment

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

There is no need to optimize the given code as it seems correct and efficient already. The functions named getLocalDir() can also be renamed to avoid conflicts with other function names, such as SearchWithPage().

The comments at lines 50 and 107 seem redundant because they repeat information within those lines; however, these redundancies do not cause errors in the existing code but serve a stylistic purpose rather than improving efficiency. If you find them overly verbose and wish to reduce redundancy without altering functionality, consider updating the variable name, comment header text slightly, or commenting out the additional line entirely:

func (u *BackupService) getLocalDir()
   = func (*BackupService) (localDir string, error error){ } +  
   = func (&BackupService)(localDir string, error error){ return }

if err := u.getLocalDir(); err != nil { ... }

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Dec 26, 2024

Choose a reason for hiding this comment

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

There are no apparent inconsistencies between the original code snippet provided and the current version of it.

This is good practice to keep files up-to-date with the latest changes. It ensures that new features or fixes do not get lost in older versions when looking at different parts of the same codebase.

Expand Down
6 changes: 3 additions & 3 deletions frontend/src/api/modules/backup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { Backup } from '../interface/backup';
import { TimeoutEnum } from '@/enums/http-enum';

// backup-agent
export const listBackupOptions = () => {
return http.get<Array<Backup.BackupOption>>(`/backups/options`);
};
export const handleBackup = (params: Backup.Backup) => {
return http.post(`/backups/backup`, params, TimeoutEnum.T_1H);
};
export const listBackupOptions = () => {
return http.get<Array<Backup.BackupOption>>(`/backups/options`);
};
export const handleRecover = (params: Backup.Recover) => {
return http.post(`/backups/recover`, params, TimeoutEnum.T_1D);
};
Expand Down
Loading