Skip to content

Commit

Permalink
removed caching of configmanager, expect a lot of test cases that nee…
Browse files Browse the repository at this point in the history
…d to be fixed

Signed-off-by: Jeromy Cannon <[email protected]>
  • Loading branch information
jeromy-cannon committed Oct 18, 2024
1 parent 3d868e6 commit 98e1e9f
Show file tree
Hide file tree
Showing 15 changed files with 55 additions and 271 deletions.
8 changes: 2 additions & 6 deletions src/commands/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -798,9 +798,9 @@ export class NodeCommand extends BaseCommand {
const self = this
if (localBuildPath !== '') {
return self.uploadPlatformSoftware(nodeAliases, podNames, task, localBuildPath)
}
}
return self.fetchPlatformSoftware(nodeAliases, podNames, releaseTag, task, self.platformInstaller)

}

fetchPlatformSoftware (nodeAliases: NodeAliases, podNames: Record<NodeAlias, PodName>, releaseTag: string,
Expand Down Expand Up @@ -1214,7 +1214,6 @@ export class NodeCommand extends BaseCommand {
// reset flags so that keys are not regenerated later
self.configManager.setFlag(flags.generateGossipKeys, false)
self.configManager.setFlag(flags.generateTlsKeys, false)
self.configManager.persist()
}
}
])
Expand Down Expand Up @@ -1844,7 +1843,6 @@ export class NodeCommand extends BaseCommand {
// reset flags so that keys are not regenerated later
self.configManager.setFlag(flags.generateGossipKeys, false)
self.configManager.setFlag(flags.generateTlsKeys, false)
self.configManager.persist()
}
}
]
Expand Down Expand Up @@ -2652,7 +2650,6 @@ export class NodeCommand extends BaseCommand {
// reset flags so that keys are not regenerated later
self.configManager.setFlag(flags.generateGossipKeys, false)
self.configManager.setFlag(flags.generateTlsKeys, false)
self.configManager.persist()
}
}
], {
Expand Down Expand Up @@ -2869,7 +2866,6 @@ export class NodeCommand extends BaseCommand {
// reset flags so that keys are not regenerated later
self.configManager.setFlag(flags.generateGossipKeys, false)
self.configManager.setFlag(flags.generateTlsKeys, false)
self.configManager.persist()
}
}
]
Expand Down
2 changes: 0 additions & 2 deletions src/commands/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,6 @@ export async function execute (task: ListrTaskWrapper<any, any, any>, configMana
const input = await prompt(task, configManager.getFlag(flag))
configManager.setFlag(flag, input)
}

configManager.persist()
}

/**
Expand Down
2 changes: 0 additions & 2 deletions src/commands/relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ export class RelayCommand extends BaseCommand {

// reset nodeAlias
self.configManager.setFlag(flags.nodeAliasesUnparsed, '')
self.configManager.persist()
}
},
{
Expand Down Expand Up @@ -316,7 +315,6 @@ export class RelayCommand extends BaseCommand {

// reset nodeAliasesUnparsed
self.configManager.setFlag(flags.nodeAliasesUnparsed, '')
self.configManager.persist()
},
skip: (ctx) => !ctx.config.isChartInstalled
}
Expand Down
47 changes: 3 additions & 44 deletions src/core/config_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,11 @@
* limitations under the License.
*
*/
import fs from 'fs'
import { SoloError, MissingArgumentError } from './errors.ts'
import { constants } from './index.ts'
import { SoloLogger } from './logging.ts'
import * as flags from '../commands/flags.ts'
import * as paths from 'path'
import * as helpers from './helpers.ts'
import * as yaml from 'js-yaml'
import { yamlToObject } from './helpers.ts'
import type * as yargs from 'yargs'
import { type CommandFlag } from '../types/index.js'

Expand All @@ -35,26 +31,12 @@ import { type CommandFlag } from '../types/index.js'
export class ConfigManager {
config!: Record<string, any>

constructor (private readonly logger: SoloLogger, private readonly cachedConfigFile = constants.SOLO_CONFIG_FILE) {
constructor (private readonly logger: SoloLogger) {
if (!logger || !(logger instanceof SoloLogger)) throw new MissingArgumentError('An instance of core/SoloLogger is required')
if (!cachedConfigFile) throw new MissingArgumentError('cached config file path is required')

this.reset()
}

/**
* Load the cached config
*/
load () {
try {
if (fs.existsSync(this.cachedConfigFile)) {
this.config = yamlToObject(this.cachedConfigFile) as Record<string, any>
}
} catch (e: Error | any) {
throw new SoloError(`failed to initialize config manager: ${e.message}`, e)
}
}

/** Reset config */
reset () {
this.config = {
Expand All @@ -69,8 +51,7 @@ export class ConfigManager {
*
* It uses the below precedence for command flag values:
* 1. User input of the command flag
* 2. Cached config value of the command flag.
* 3. Default value of the command flag if the command is not 'init'.
* 2. Default value of the command flag if the command is not 'init'.
*/
applyPrecedence (argv: yargs.Argv<any>, aliases: any): yargs.Argv<any> {
for (const key of Object.keys(aliases)) {
Expand All @@ -93,7 +74,7 @@ export class ConfigManager {
}

/** Update the config using the argv */
update (argv: object | any = {}, persist: boolean = false) {
update (argv: object | any = {}) {
if (argv && Object.keys(argv).length > 0) {
for (const flag of flags.allFlags) {
if (flag.name === flags.force.name) {
Expand Down Expand Up @@ -147,23 +128,6 @@ export class ConfigManager {
}

this.config.updatedAt = new Date().toISOString()

if (persist) {
this.persist()
}
}
}

/** Persist the config in the cached config file */
persist () {
try {
this.config.updatedAt = new Date().toISOString()
const newYaml = yaml.dump(this.config)
fs.writeFileSync(this.cachedConfigFile, newYaml)
// refresh config with the file contents
this.load()
} catch (e: Error | any) {
throw new SoloError(`failed to persis config: ${e.message}`, e)
}
}

Expand Down Expand Up @@ -194,9 +158,4 @@ export class ConfigManager {
getVersion (): string {
return this.config.version
}

/** Get last updated at timestamp */
getUpdatedAt (): string {
return this.config.updatedAt
}
}
1 change: 0 additions & 1 deletion src/core/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export const SOLO_CACHE_DIR = path.join(SOLO_HOME_DIR, 'cache')
export const SOLO_VALUES_DIR = path.join(SOLO_CACHE_DIR, 'values-files')
export const DEFAULT_NAMESPACE = 'default'
export const HELM = 'helm'
export const SOLO_CONFIG_FILE = path.join(SOLO_HOME_DIR, 'solo.yaml')
export const RESOURCES_DIR = normalize(path.join(ROOT_DIR, 'resources'))
export const TEMP_DIR = normalize(path.join(ROOT_DIR, 'temp'))

Expand Down
34 changes: 18 additions & 16 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function main (argv: any) {
const zippy = new Zippy(logger)
const helmDepManager = new HelmDependencyManager(downloader, zippy, logger)
const depManagerMap = new Map()
.set(constants.HELM, helmDepManager)
.set(constants.HELM, helmDepManager)
const depManager = new DependencyManager(logger, depManagerMap)

const helm = new Helm(logger)
Expand Down Expand Up @@ -79,7 +79,9 @@ export function main (argv: any) {
}

const processArguments = (argv: any, yargs: any) => {
argv._[0] === 'init' ? configManager.reset() : configManager.load()
if (argv._[0] === 'init') {
configManager.reset()
}

// Set default cluster name and namespace from kubernetes context
// these will be overwritten if user has entered the flag values explicitly
Expand All @@ -92,7 +94,7 @@ export function main (argv: any) {
argv = configManager.applyPrecedence(argv, yargs.parsed.aliases)

// update and persist config
configManager.update(argv, true)
configManager.update(argv)

logger.showUser(chalk.cyan('\n******************************* Solo *********************************************'))
logger.showUser(chalk.cyan('Version\t\t\t:'), chalk.yellow(configManager.getVersion()))
Expand All @@ -105,19 +107,19 @@ export function main (argv: any) {
}

return yargs(hideBin(argv))
.usage('Usage:\n $0 <command> [options]')
.alias('h', 'help')
.alias('v', 'version')
// @ts-ignore
.command(commands.Initialize(opts))
.strict()
// @ts-ignore
.option(flags.devMode.name, flags.devMode.definition)
.wrap(120)
.demand(1, 'Select a command')
// @ts-ignore
.middleware(processArguments, false) // applyBeforeValidate = false as otherwise middleware is called twice
.parse()
.usage('Usage:\n $0 <command> [options]')
.alias('h', 'help')
.alias('v', 'version')
// @ts-ignore
.command(commands.Initialize(opts))
.strict()
// @ts-ignore
.option(flags.devMode.name, flags.devMode.definition)
.wrap(120)
.demand(1, 'Select a command')
// @ts-ignore
.middleware(processArguments, false) // applyBeforeValidate = false as otherwise middleware is called twice
.parse()
} catch (e: Error | any) {
logger.showUserError(e)
process.exit(1)
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/commands/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ e2eTestSuite(testName, argv, undefined, undefined, undefined, undefined, undefin
try {
argv[flags.privateKey.name] = constants.GENESIS_KEY
argv[flags.amount.name] = 777
configManager.update(argv, true)
configManager.update(argv)

await expect(accountCmd.create(argv)).to.eventually.be.ok

Expand All @@ -166,7 +166,7 @@ e2eTestSuite(testName, argv, undefined, undefined, undefined, undefined, undefin
try {
argv[flags.amount.name] = 0
argv[flags.accountId.name] = accountId1
configManager.update(argv, true)
configManager.update(argv)

await expect(accountCmd.update(argv)).to.eventually.be.ok

Expand All @@ -188,7 +188,7 @@ e2eTestSuite(testName, argv, undefined, undefined, undefined, undefined, undefin
argv[flags.accountId.name] = accountId2
argv[flags.privateKey.name] = constants.GENESIS_KEY
argv[flags.amount.name] = 333
configManager.update(argv, true)
configManager.update(argv)

await expect(accountCmd.update(argv)).to.eventually.be.ok

Expand All @@ -208,7 +208,7 @@ e2eTestSuite(testName, argv, undefined, undefined, undefined, undefined, undefin
it('should be able to get account-1', async () => {
try {
argv[flags.accountId.name] = accountId1
configManager.update(argv, true)
configManager.update(argv)

await expect(accountCmd.get(argv)).to.eventually.be.ok
// @ts-ignore to access the private property
Expand All @@ -227,7 +227,7 @@ e2eTestSuite(testName, argv, undefined, undefined, undefined, undefined, undefin
it('should be able to get account-2', async () => {
try {
argv[flags.accountId.name] = accountId2
configManager.update(argv, true)
configManager.update(argv)

await expect(accountCmd.get(argv)).to.eventually.be.ok
// @ts-ignore to access the private property
Expand All @@ -249,7 +249,7 @@ e2eTestSuite(testName, argv, undefined, undefined, undefined, undefined, undefin
try {
argv[flags.ecdsaPrivateKey.name] = ecdsaPrivateKey.toString()
argv[flags.setAlias.name] = true
configManager.update(argv, true)
configManager.update(argv)

await expect(accountCmd.create(argv)).to.eventually.be.ok

Expand Down
10 changes: 5 additions & 5 deletions test/e2e/commands/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('ClusterCommand', () => {

await k8.deleteNamespace(namespace)
argv[flags.clusterSetupNamespace.name] = constants.SOLO_SETUP_NAMESPACE
configManager.update(argv, true)
configManager.update(argv)
await clusterCmd.setup(argv) // restore solo-cluster-setup for other e2e tests to leverage
do {
await sleep(5 * SECONDS)
Expand All @@ -85,13 +85,13 @@ describe('ClusterCommand', () => {

it('solo cluster setup should fail with invalid cluster name', async () => {
argv[flags.clusterSetupNamespace.name] = 'INVALID'
configManager.update(argv, true)
configManager.update(argv)
await expect(clusterCmd.setup(argv)).to.be.rejectedWith('Error on cluster setup')
}).timeout(MINUTES)

it('solo cluster setup should work with valid args', async () => {
argv[flags.clusterSetupNamespace.name] = namespace
configManager.update(argv, true)
configManager.update(argv)
await expect(clusterCmd.setup(argv)).to.eventually.be.ok
}).timeout(MINUTES)

Expand All @@ -111,7 +111,7 @@ describe('ClusterCommand', () => {
// helm list would return an empty list if given invalid namespace
it('solo cluster reset should fail with invalid cluster name', async () => {
argv[flags.clusterSetupNamespace.name] = 'INVALID'
configManager.update(argv, true)
configManager.update(argv)

try {
await expect(clusterCmd.reset(argv)).to.be.rejectedWith('Error on cluster reset')
Expand All @@ -123,7 +123,7 @@ describe('ClusterCommand', () => {

it('solo cluster reset should work with valid args', async () => {
argv[flags.clusterSetupNamespace.name] = namespace
configManager.update(argv, true)
configManager.update(argv)
await expect(clusterCmd.reset(argv)).to.eventually.be.ok
}).timeout(MINUTES)
})
2 changes: 1 addition & 1 deletion test/e2e/commands/network.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('NetworkCommand', () => {
argv[flags.deletePvcs.name] = true
argv[flags.deleteSecrets.name] = true
argv[flags.force.name] = true
configManager.update(argv, true)
configManager.update(argv)

try {
await expect(networkCmd.destroy(argv)).to.eventually.be.ok
Expand Down
10 changes: 4 additions & 6 deletions test/e2e/e2e_node_util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,15 @@ import {
balanceQueryShouldSucceed,
e2eTestSuite,
getDefaultArgv,
getTestConfigManager,
HEDERA_PLATFORM_VERSION_TAG,
TEST_CLUSTER
TEST_CLUSTER, testLogger
} from '../test_util.ts'
import { getNodeLogs, sleep } from '../../src/core/helpers.ts'
import { NodeCommand } from '../../src/commands/node.ts'
import { MINUTES, SECONDS } from '../../src/core/constants.ts'
import type { NodeAlias } from '../../src/types/aliases.ts'
import { NodeAliases } from '../../src/types/aliases.ts'
import type { ListrTaskWrapper } from 'listr2'
import type { K8 } from '../../src/core/index.ts'
import { ConfigManager, type K8 } from '../../src/core/index.ts'

export function e2eNodeKeyRefreshTest (testName: string, mode: string, releaseTag = HEDERA_PLATFORM_VERSION_TAG) {
const namespace = testName
Expand Down Expand Up @@ -174,8 +172,8 @@ export function e2eNodeKeyRefreshTest (testName: string, mode: string, releaseTa

async function nodeRefreshTestSetup (argv: Record<any, any>, testName: string, k8: K8, nodeAliases: string) {
argv[flags.nodeAliasesUnparsed.name] = nodeAliases
const configManager = getTestConfigManager(`${testName}-solo.yaml`)
configManager.update(argv, true)
const configManager = new ConfigManager(testLogger)
configManager.update(argv)

const podArray = await k8.getPodsByLabel(
[`app=network-${nodeAliases}`,
Expand Down
2 changes: 0 additions & 2 deletions test/e2e/integration/core/platform_installer_e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ e2eTestSuite(namespace, argv, undefined, undefined, undefined, undefined, undefi
describe('PackageInstallerE2E', async () => {
const k8 = bootstrapResp.opts.k8
const accountManager = bootstrapResp.opts.accountManager
const configManager = bootstrapResp.opts.configManager
const installer = bootstrapResp.opts.platformInstaller
const podName = 'network-node1-0'
const packageVersion = 'v0.42.5'
Expand All @@ -68,7 +67,6 @@ e2eTestSuite(namespace, argv, undefined, undefined, undefined, undefined, undefi
if (!fs.existsSync(testCacheDir)) {
fs.mkdirSync(testCacheDir)
}
configManager.load()
})

describe('fetchPlatform', () => {
Expand Down
Loading

0 comments on commit 98e1e9f

Please sign in to comment.