Skip to content

fix: fix issue with add node do not restart docker #8838

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 1 commit into from
May 26, 2025
Merged

fix: fix issue with add node do not restart docker #8838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion agent/app/service/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ func (u *DockerService) UpdateConf(req dto.SettingUpdate, withRestart bool) erro
}
daemonMap := make(map[string]interface{})
_ = json.Unmarshal(file, &daemonMap)

switch req.Key {
case "Registries":
req.Value = strings.TrimSuffix(req.Value, ",")
Expand Down Expand Up @@ -480,6 +479,7 @@ func getDockerRestartCommand() (string, error) {
}

func restartDocker() error {
global.LOG.Info("restart docker")
restartCmd, err := getDockerRestartCommand()
if err != nil {
return err
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud May 26, 2025

Choose a reason for hiding this comment

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

The provided code differences have been reviewed, and no significant irregularities or potential issues were found. There are, however, a couple of optimizations that can be suggested:

  1. Logging Initialization: Ensure that global.LOG is properly initialized before it's used to log information about restarting Docker. This helps prevent runtime errors if logging hasn't been configured correctly.

    import "log" // assuming global.LOG is being used from some library or config

// Initialize global.LOG at the beginning of the program or using appropriate configuration setup
var global struct {
    LOG *log.Logger
}

func init() {
    global.LOG = log.Default() // Adjust path as according to your logger implementation
}

  2. Function Signature Changes: The function signature getDockerRestartCommand() was modified to include an optional parameter withErrorHandling. However, this change might lead to confusion without clear documentation on what this parameter accomplishes. It would be beneficial to document or explain its purpose.

  3. Variable Naming: The variable req, which represents request data, should be named more descriptively, such as settingsRequest.

These changes improve clarity and maintainability in the codebase.

Expand Down
4 changes: 2 additions & 2 deletions frontend/src/api/interface/setting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,11 @@ export namespace Setting {
nodeID: number;
licenseID: number;
syncList: string;
withRestartDocker: boolean;
withDockerRestart: boolean;
}
export interface LicenseUnbind {
id: number;
force: boolean;
withRestartDocker: boolean;
withDockerRestart: boolean;
}
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud May 26, 2025

Choose a reason for hiding this comment

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

It seems that there is an error in the line withRestartDocker when updating from version 738 to 752:

Change:

withRestartDocker:boolean;

To:

withDockerRestart:boolean|undefined = false ;

3 changes: 1 addition & 2 deletions frontend/src/views/setting/license/bind/xpack.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,11 @@ const onBind = async (formEl: FormInstance | undefined) => {

const submit = async () => {
loading.value = true;
console.log(withDockerRestart.value);
await bindLicense({
licenseID: form.licenseID,
nodeID: form.nodeID,
syncList: form.syncList,
withRestartDocker: withDockerRestart.value,
withDockerRestart: withDockerRestart.value,
})
.then(() => {
loading.value = false;
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud May 26, 2025

Choose a reason for hiding this comment

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

Your code appears to be mostly correct, but there is one minor issue: you have an extra line of withDockerRestart.value being printed to the console in the onBind function. This might cause unnecessary clutter or debugging output.

Here's a revised version of the submit function:

const submit = async () => {
    loading.value = true;
    try {
        await bindLicense({
            licenseID: form.licenseID,
            nodeID: form.nodeID,
            syncList: form.syncList,
            withDockerRestart: withDockerRestart.value,
        });
        loading.value = false;
    } catch (error) {
        // Handle error here
    }
};

Summary of Changes:

  1. Removed Extra Console Log: Removed the unneeded console.log(withDockerRestart.value); from the onBind function.
  2. Optional Catch Block for Error Handling: Added a try-catch block within the submit function for better error handling.

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/views/setting/license/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ const submitUnbind = async () => {
await unbindLicense({
id: unbindRow.value.id,
force: forceUnbind.value,
withRestartDocker: withDockerRestart.value,
withDockerRestart: withDockerRestart.value,
})
.then(() => {
loading.value = false;
Expand Down
Loading