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.

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

HA: multiple instance replicas #85

Merged
merged 15 commits into from Jun 2, 2023
Merged

HA: multiple instance replicas #85

merged 15 commits into from Jun 2, 2023

Conversation

hardillb
Copy link
Contributor

@hardillb hardillb commented May 25, 2023

Description

Sets the correct number of replicas on the Deployment object

Also has support for merging the logs from multiple replicas

To do:

  • send commands to all instances

Related Issue(s)

FlowFuse/flowfuse#2156

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

When HA enabled call all the endpoints to make sure all
instances stay in sync
@hardillb hardillb requested a review from knolleary May 31, 2023 14:13
@hardillb hardillb marked this pull request as ready for review May 31, 2023 14:13
@hardillb hardillb self-assigned this May 31, 2023
kubernetes.js Outdated
@@ -306,6 +306,12 @@ const createDeployment = async (project, options) => {
localPod.spec.containers[0].resources.limits.cpu = `${stack.cpu * 10}m`
}

const ha = await project.getSetting('ha')
console.log(ha)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.log(ha)
console.log(ha)

Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

Some minor tidy up of commented out code - and an observation on the status endpoint. Would like a follow-up task to be raised to review gathering status of all instances, not just one.

@@ -727,6 +748,7 @@ module.exports = {
(details.body.status?.conditions[0].type === 'Available' ||
(details.body.status?.conditions[0].type === 'Progressing' && details.body.status?.conditions[0].reason === 'NewReplicaSetAvailable')
)) {
// not calling all endpoints for HA as they should be the same
Copy link
Member

Choose a reason for hiding this comment

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

This feels a little risky - in case one instance has hit a hang or other unpleasantness that goes unnoticed.

For an initial iteration, we can go with this, but will need to look at next week.

kubernetes.js Outdated Show resolved Hide resolved
kubernetes.js Outdated
Comment on lines 883 to 902
// const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''
// if (await project.getSetting('ha')) {
// const endpoints = await this._k8sApi.readNamespacedEndpoints(`${prefix}${project.safeName}`, this._namespace)
// const addresses = endpoints.body.subsets[0].addresses.map(a => { return a.ip })
// const commands = []
// for (const address in addresses) {
// commands.push(got.post(`http://${addresses[address]}:2880/flowforge/command`, {
// json: {
// cmd: 'stop'
// }
// }))
// }
// await Promise.all(commands)
// } else {
// await got.post(`http://${prefix}${project.safeName}.${this._namespace}:2880/flowforge/command`, {
// json: {
// cmd: 'stop'
// }
// })
// }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''
// if (await project.getSetting('ha')) {
// const endpoints = await this._k8sApi.readNamespacedEndpoints(`${prefix}${project.safeName}`, this._namespace)
// const addresses = endpoints.body.subsets[0].addresses.map(a => { return a.ip })
// const commands = []
// for (const address in addresses) {
// commands.push(got.post(`http://${addresses[address]}:2880/flowforge/command`, {
// json: {
// cmd: 'stop'
// }
// }))
// }
// await Promise.all(commands)
// } else {
// await got.post(`http://${prefix}${project.safeName}.${this._namespace}:2880/flowforge/command`, {
// json: {
// cmd: 'stop'
// }
// })
// }

kubernetes.js Outdated
Comment on lines 951 to 970
// const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''
// if (project.getSetting('ha')) {
// const endpoints = await this._k8sApi.readNamespacedEndpoints(`${prefix}${project.safeName}`, this._namespace)
// const addresses = endpoints.body.subsets[0].addresses.map(a => { return a.ip })
// const commands = []
// for (const address in addresses) {
// commands.push(got.post(`http://${addresses[address]}:2880/flowforge/command`, {
// json: {
// cmd: 'restart'
// }
// }))
// }
// await Promise.all(commands)
// } else {
// await got.post(`http://${prefix}${project.safeName}.${this._namespace}:2880/flowforge/command`, {
// json: {
// cmd: 'restart'
// }
// })
// }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''
// if (project.getSetting('ha')) {
// const endpoints = await this._k8sApi.readNamespacedEndpoints(`${prefix}${project.safeName}`, this._namespace)
// const addresses = endpoints.body.subsets[0].addresses.map(a => { return a.ip })
// const commands = []
// for (const address in addresses) {
// commands.push(got.post(`http://${addresses[address]}:2880/flowforge/command`, {
// json: {
// cmd: 'restart'
// }
// }))
// }
// await Promise.all(commands)
// } else {
// await got.post(`http://${prefix}${project.safeName}.${this._namespace}:2880/flowforge/command`, {
// json: {
// cmd: 'restart'
// }
// })
// }

kubernetes.js Outdated
Comment on lines 991 to 1018
// const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''
// if (project.getSetting('ha')) {
// const endpoints = await this._k8sApi.readNamespacedEndpoints(`${prefix}${project.safeName}`, this._namespace)
// const addresses = endpoints.body.subsets[0].addresses.map(a => { return a.ip })
// const commands = []
// for (const address in addresses) {
// commands.push(got.post(`http://${addresses[address]}:2880/flowforge/command`, { // logout:nodered(step-4)
// json: {
// cmd: 'logout',
// token
// }
// }))
// }
// Promise.all(commands).catch(error => {
// this._app.log.error(`[k8s] Project ${project.id} - error in 'revokeUserToken': ${error.stack}`)
// })
// } else {
// try {
// await got.post(`http://${prefix}${project.safeName}.${this._namespace}:2880/flowforge/command`, { // logout:nodered(step-4)
// json: {
// cmd: 'logout',
// token
// }
// })
// } catch (error) {
// this._app.log.error(`[k8s] Project ${project.id} - error in 'revokeUserToken': ${error.stack}`)
// }
// }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''
// if (project.getSetting('ha')) {
// const endpoints = await this._k8sApi.readNamespacedEndpoints(`${prefix}${project.safeName}`, this._namespace)
// const addresses = endpoints.body.subsets[0].addresses.map(a => { return a.ip })
// const commands = []
// for (const address in addresses) {
// commands.push(got.post(`http://${addresses[address]}:2880/flowforge/command`, { // logout:nodered(step-4)
// json: {
// cmd: 'logout',
// token
// }
// }))
// }
// Promise.all(commands).catch(error => {
// this._app.log.error(`[k8s] Project ${project.id} - error in 'revokeUserToken': ${error.stack}`)
// })
// } else {
// try {
// await got.post(`http://${prefix}${project.safeName}.${this._namespace}:2880/flowforge/command`, { // logout:nodered(step-4)
// json: {
// cmd: 'logout',
// token
// }
// })
// } catch (error) {
// this._app.log.error(`[k8s] Project ${project.id} - error in 'revokeUserToken': ${error.stack}`)
// }
// }

@knolleary knolleary merged commit c650b6a into main Jun 2, 2023
1 check passed
@knolleary knolleary deleted the 2156-ha-replicas branch June 2, 2023 10:27
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.

None yet

2 participants