Skip to content
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

Jump to bottom

fix(server): rm minio alias init in boot, fix multi region limit; #976

Merged
merged 1 commit into from
Mar 29, 2023
Merged

fix(server): rm minio alias init in boot, fix multi region limit; #976

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/account/account.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class AccountService {
orderNumber: string,
amount: number,
currency: Currency,
description = 'Account charge',
description = 'laf account charge',
) {
// webchat pay
if (channel === PaymentChannelType.WeChat) {
maslow marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion server/src/app.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { RegionModule } from './region/region.module'
import { GatewayModule } from './gateway/gateway.module'
import { PrismaModule } from './prisma/prisma.module'
import { SubscriptionModule } from './subscription/subscription.module'
import { AccountModule } from './account/account.module';
import { AccountModule } from './account/account.module'

@Module({
imports: [
maslow marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
27 changes: 18 additions & 9 deletions server/src/database/database.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ export class DatabaseService {

async findOne(appid: string) {
const database = await this.prisma.database.findUnique({
where: {
appid,
},
where: { appid },
})

return database
Expand All @@ -72,15 +70,14 @@ export class DatabaseService {

// delete app database in database
const doc = await this.prisma.database.delete({
where: {
appid: database.appid,
},
where: { appid: database.appid },
})

return doc
}

getConnectionUri(region: Region, database: Database) {
// Get application internal database connection uri
getInternalConnectionUri(region: Region, database: Database) {
// build app db connection uri from config
const parsed = mongodb_uri.parse(region.databaseConf.connectionUri)
parsed.database = database.name
Expand All @@ -91,6 +88,18 @@ export class DatabaseService {
return mongodb_uri.format(parsed)
}

// Get application control database connection uri
getControlConnectionUri(region: Region, database: Database) {
// build app db connection uri from config
const parsed = mongodb_uri.parse(region.databaseConf.controlConnectionUri)
parsed.database = database.name
parsed.username = database.user
parsed.password = database.password
parsed.options['authSource'] = database.name

return mongodb_uri.format(parsed)
}

/**
* Get database accessor that used for `database-proxy`
*/
Expand All @@ -100,7 +109,7 @@ export class DatabaseService {
assert(database, 'Database not found')

const dbName = database.name
const connectionUri = this.getConnectionUri(region, database)
const connectionUri = this.getControlConnectionUri(region, database)
assert(connectionUri, 'Database connection uri not found')

const accessor = new MongoAccessor(dbName, connectionUri)
Expand All @@ -116,7 +125,7 @@ export class DatabaseService {
const database = await this.findOne(appid)
assert(database, 'Database not found')

const connectionUri = this.getConnectionUri(region, database)
const connectionUri = this.getControlConnectionUri(region, database)

const client = await this.mongoService.connectDatabase(
connectionUri,
maslow marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
14 changes: 0 additions & 14 deletions server/src/initializer/initializer.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,20 +163,6 @@ export class InitializerService {
return res
}

async initMinioAlias() {
const regions = await this.regionService.findAll()

for (const region of regions) {
const res = await this.minioService.setMinioClientTarget(region)
if (res.status === 'success') {
this.logger.verbose('MinioService init - ' + region.name + ' success')
} else {
this.logger.error('MinioService init - ' + region.name + ' failed', res)
throw new Error('set minio client target failed ' + region.name)
}
}
}

async createDefaultAuthProvider() {
// check if exists
const existed = await this.prisma.authProvider.count()
maslow marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion server/src/instance/instance.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class InstanceService {

// db connection uri
const database = await this.databaseService.findOne(appid)
const dbConnectionUri = this.databaseService.getConnectionUri(
const dbConnectionUri = this.databaseService.getInternalConnectionUri(
app.region,
database,
)
maslow marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
1 change: 0 additions & 1 deletion server/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ async function bootstrap() {
await initService.createDefaultBundle()
await initService.createDefaultRuntime()
await initService.createDefaultAuthProvider()
await initService.initMinioAlias()
} catch (error) {
console.error(error)
process.exit(1)
Copy link

Choose a reason for hiding this comment

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

with a brief overview of the code fragment:

This code fragment is part of the bootstrap() function, which appears to be responsible for initializing the system. The code is trying to create a default bundle, runtime, and authentication provider. The removed line of code was responsible for initializing the Minio alias.

Now, let's discuss potential bugs and improvement suggestions:

  1. Bug Risk: The code does not check for errors when creating the default bundle, runtime, and authentication provider. If any of these operations fail, the system may not properly initialize.

  2. Improvement Suggestion: Add error handling when creating the default bundle, runtime, and authentication provider to ensure that the system can gracefully handle any errors.

  3. Improvement Suggestion: Consider adding a logging mechanism to track the progress of the bootstrapping process. This will help to identify any issues quickly in the event of a failure.

Expand Down
19 changes: 17 additions & 2 deletions server/src/storage/minio/minio.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,28 @@ import * as cp from 'child_process'
import { promisify } from 'util'
import { MinioCommandExecOutput } from './types'
import { MINIO_COMMON_USER_GROUP } from 'src/constants'
import { RegionService } from 'src/region/region.service'

const exec = promisify(cp.exec)

@Injectable()
export class MinioService {
private readonly logger = new Logger(MinioService.name)

constructor(private readonly regionService: RegionService) {
this.regionService.findAll().then(async (regions) => {
for (const region of regions) {
const res = await this.setMinioClientTarget(region)
if (res.status === 'success') {
this.logger.log('minio alias init - ' + region.name + ' success')
} else {
this.logger.log('minio alias init ' + region.name + ' failed')
this.logger.debug(res)
}
}
})
}

/**
* Create s3 client
* @returns
Expand All @@ -29,7 +44,7 @@ export class MinioService {
const conf = region.storageConf

return new S3({
endpoint: conf.externalEndpoint,
endpoint: conf.controlEndpoint,
credentials: {
accessKeyId: conf.accessKey,
secretAccessKey: conf.secretKey,
Expand Down Expand Up @@ -242,7 +257,7 @@ export class MinioService {
const conf = region.storageConf
const access_key = conf.accessKey
const access_secret = conf.secretKey
const endpoint = conf.externalEndpoint
const endpoint = conf.controlEndpoint
const target = region.name

const cmd = `alias set ${target} ${endpoint} ${access_key} ${access_secret}`
Copy link

Choose a reason for hiding this comment

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

with the code review:

  1. The code looks valid and it is well written, there are no syntax errors. However, it is not formatted properly so it is difficult to read. It would be better to add indentation and line breaks for better readability.

  2. It seems that the constructor is well structured and the function calls are properly followed. However, it is not clear what the purpose of calling the "setMinioClientTarget" function is. Adding some comments to explain what it does would improve the clarity of the code.

  3. The code is missing error handling for the async calls to the regionService. It should be added to catch any potential errors that could occur.

  4. The logging statements should be improved to include more useful information. For example, instead of just logging "minio alias init - 'region name' success", the function call and any parameters should be included as well.

Expand Down