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

Syncing configuration to the new node after node add #7949

Merged
merged 50 commits into from
Jun 22, 2023

Conversation

ArvinthC3000
Copy link
Collaborator

@ArvinthC3000 ArvinthC3000 commented Jun 5, 2023

🔩 Description: What code changed, and why?

As an Automate HA user, when the user adds a new node, it should also add/sync up all the configs added in the bootstrap Frontend node.

⛓️ Related Resources

Jira ticket: https://chefio.atlassian.net/browse/CHEF-2712

👍 Definition of Done

Following are the scenarios covered in testing:

  • On-Prem with Chef-embedded - Add node.
  • On-Prem with Chef-embedded - Remove node (x) Need to check.
  • On-prem with Chef-embedded and custom cert - Add node.
  • On-prem with Chef-embedded and custom cert - Remove node.
  • On-prem with Manged services - Add/Remove node.
  • Any Infra with Chef-embedded - Add/Remove OS and PG node.
  • AWS with Chef-embedded - Add node.
  • AWS with Chef-embedded - Remove the bootstrap node (x) Not working.
  • AWS with Managed AWS - Add node.
  • AWS with Managed AWS Remove the bootstrap node and check whether the config is still intact.

Note:

  • Sync-up will happen only in frontend nodes, as the BE nodes run in hab cluster
  • Sync-up will ignore the load balance tls values as that will cause issue in case the HA setup uses unique cert in each node.

Why testing on Remove node flow?

Because of the following behavior of nodes getting replaced, the sync up is done in remove node flow so that the newly added nod will also have all config as well.
Before removing nodes. Automate nodes:

Service IP Address Status Opensearch  
automate 10.0.192.105 OK green (Active: 100.0)  
automate 10.0.192.159 OK green (Active: 100.0)  
automate 10.0.192.222 OK green (Active: 100.0) <- removing this node with 'node remove' command (index:2)
automate 10.0.192.90 OK green (Active: 100.0)  
automate 10.0.192.164 OK green (Active: 100.0) <- This is being replaced (remove and create resource) from index:4 to index:2

After remove node:

Service IP Address Status Opensearch  
automate 10.0.192.105 OK green (Active: 100.0)  
automate 10.0.192.159 OK green (Active: 100.0)  
automate 10.0.192.212 OK green (Active: 100.0) . <- newly added node does not have dex config (this is replacement of node in index:4 before removing node )
automate 10.0.192.90 OK green (Active: 100.0)  

👟 How to Build and Test the Change

✅ Checklist

All PRs must tick these:

With occasional exceptions, all PRs from Progress employees must tick these:

  • Is the code clear? (complicated code or lots of comments--subdivide and use well-named methods, meaningful variable names, etc.)
  • Consistency checked? (user notifications, user prompts, visual patterns, code patterns, variable names)
  • Repeated code blocks eliminated? (adapt and reuse existing components, blocks, functions, etc.)
  • Spelling, grammar, typos checked? (at a minimum use make spell in any component directory)
  • Code well-formatted? (indents, line breaks, etc. improve rather than hinder readability)

All PRs from Progress employees should tick these if appropriate:

  • Tests added/updated? (all new code needs new tests)
  • Docs added/updated? (all customer-facing changes)

Please add a note next to any checkbox above if you are NOT ticking it.

📷 Screenshots, if applicable

Demo Videos:
Add node in AWS:
https://progresssoftware.sharepoint.com/:v:/s/ChefCoreC/ESX5RAGEcJxDiS8JjKc4HgQBjP9rHKRU-v_HSZ9voEG-sQ?e=Ejn8xR

Add node in On-prem Managed services:
https://progresssoftware.sharepoint.com/:v:/s/ChefCoreC/Ef7rnGtWzgtPk-WZlSSB2-kB88vaVmopRaynYB2f5kCfTw?e=WKsGbw

Docs and screenshots:
Config of Newly Added node without this PR change: https://progresssoftware-my.sharepoint.com/:u:/g/personal/archand_progress_com/EagitM3p-NtPhWfHg-ZBLh8B8-OTYU35dzn5F8e_q5vfmg?e=qy9v45

Config of Newly Added node with this PR change: https://progresssoftware-my.sharepoint.com/:u:/g/personal/archand_progress_com/EYOaHHNXdHVBmcIsIVlvolcB3vvSIqvHpKtuOl_XN-3cvw?e=lmslsy

Service status after adding node and applying the new config
image

Test coverage: (Overall test coverage on Sonarqube for code written in this PR: 86%)
image

@netlify
Copy link

netlify bot commented Jun 5, 2023

👷 Deploy Preview for chef-automate processing.

Name Link
🔨 Latest commit fdd981a
🔍 Latest deploy log https://app.netlify.com/sites/chef-automate/deploys/6493e9ed5dcfc2000853b83a

@ArvinthC3000 ArvinthC3000 force-pushed the ar/config_sync-up branch 2 times, most recently from 8d8cb33 to f618496 Compare June 5, 2023 17:21
@ArvinthC3000 ArvinthC3000 changed the title [WIP]Syncing configuration to the new node after node add [Review but don't merge][TIP]Syncing configuration to the new node after node add Jun 6, 2023
@ArvinthC3000 ArvinthC3000 changed the title [Review but don't merge][TIP]Syncing configuration to the new node after node add [Review but don't merge][WIP]Syncing configuration to the new node after node add Jun 7, 2023
@ArvinthC3000 ArvinthC3000 self-assigned this Jun 7, 2023
}

// Execute 'config show' command in specific service and fetch the output file to bastion
func ExecuteCustomCmdInServiceNode(outputFiles []string, inputFiles []string, inputFilesPrefix string, service string, cmdString string, singleNode bool) error {
Copy link
Collaborator

@vivek-yadav vivek-yadav Jun 7, 2023

Choose a reason for hiding this comment

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

Suggested change
func ExecuteCustomCmdInServiceNode(outputFiles []string, inputFiles []string, inputFilesPrefix string, service string, cmdString string, singleNode bool) error {
func ExecuteCustomCmdOnEachNodeType(outputFiles []string, inputFiles []string, inputFilesPrefix string, service string, cmdString string, singleNode bool) error {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -450,7 +465,7 @@ func appendChildFileToParentFile(hostIp, parent, child string) (string, error) {
}

fileContent := string(output)
start := fmt.Sprintf("Output for Host IP %s : \n%s\n", hostIp, fileContent)
start := fmt.Sprintf("%s\n", fileContent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is needed in other commands, don't remove Host IP

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done


}
*/
func ExecuteCmdInAllNodeAndCaptureOutput(nodeObjects []*NodeObject, singleNode bool, outputDirectory string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

move this to ha_node_operations_utils.go
Also rename this, with proper explanation name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

} else {
cmd.CmdInputs.Args = inputFiles
if len(inputFiles) > 0 {
cmd.PreExec = prePatchCheckForFrontendNodes
Copy link
Collaborator

Choose a reason for hiding this comment

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

you cannot import function from lib which uses this utility

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the dependency from other libs

case OPENSEARCH:
nodeMap.Opensearch = cmd
}
infra, err := getAutomateHAInfraDetails()
Copy link
Collaborator

Choose a reason for hiding this comment

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

getAutomateHAInfraDetails is not in util, don't use it here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

}

// Create *Cmd struct instance with 'cmdString' and 'outputFiles' params
func ServiceNodeConstructor(nodeMap *NodeTypeAndCmd, cmdString string, outputFiles []string, singleNode bool) *Cmd {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func ServiceNodeConstructor(nodeMap *NodeTypeAndCmd, cmdString string, outputFiles []string, singleNode bool) *Cmd {
func NewNodeTypeCmd(nodeMap *NodeTypeAndCmd, cmdString string, outputFiles []string, singleNode bool) *Cmd {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

if err != nil {
return err
}
err = syncConfigToAllNodes()
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should run this before error check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

}
err = syncConfigToAllNodes()
if err != nil {
return err
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrap all errors with distinct strings

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
Signed-off-by: Arvinth C <arvinth.chandrasekaran@progress.com>
@sonarqube-for-infrastructure-prod

SonarQube Quality Gate

Quality Gate failed

Failed condition 22.7% 22.7% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

@punitmundra punitmundra merged commit 95b7916 into main Jun 22, 2023
5 of 8 checks passed
@punitmundra punitmundra deleted the ar/config_sync-up branch June 22, 2023 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet