-
Notifications
You must be signed in to change notification settings - Fork 489
Adds expected file to import config command #6342
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
61147a8
fe37d59
a40f12e
d214ee0
a73c921
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,8 +18,15 @@ | |
| */ | ||
| package org.apache.accumulo.server.conf.util; | ||
|
|
||
| import java.io.BufferedInputStream; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.UncheckedIOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.ArrayList; | ||
| import java.util.ConcurrentModificationException; | ||
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -44,6 +51,7 @@ | |
| import org.apache.accumulo.start.spi.KeywordExecutable; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.yaml.snakeyaml.LoaderOptions; | ||
| import org.yaml.snakeyaml.Yaml; | ||
|
|
||
| import com.beust.jcommander.JCommander; | ||
|
|
@@ -77,6 +85,12 @@ public String description() { | |
| } | ||
|
|
||
| public static class Opts extends ServerOpts { | ||
| @Parameter(names = "--input", | ||
| description = "Yaml file containing configuration data. If not specified will read from stdin.") | ||
| public String inputFile; | ||
| @Parameter(names = "--expected", | ||
| description = "Yaml file containing expected current config. Changes are only made if config in zookeeper matches whats in this file.") | ||
| public String expectedFile; | ||
| @Parameter(names = "--ignore-extra", | ||
| description = "Proceed when Accumulo has extra tables, resource groups, or namespaces that are not in yaml") | ||
| public boolean ignoreExtra = false; | ||
|
|
@@ -99,6 +113,9 @@ static PropStoreKey getKey(Scope scope, String name, ServerContext context) { | |
| } | ||
|
|
||
| record ScopeName(Scope scope, String name) { | ||
| public ScopeName(ScopedProperties sp) { | ||
| this(sp.scope(), sp.name()); | ||
| } | ||
| } | ||
|
|
||
| private static Set<ScopeName> getAllScopeNames(ServerContext context) { | ||
|
|
@@ -124,9 +141,14 @@ private static Set<ScopeName> getAllScopeNames(ServerContext context) { | |
| } | ||
|
|
||
| private static void validate(ServerContext serverContext, List<ScopedProperties> allProps, | ||
| boolean ignoreExtra) { | ||
| Map<ScopeName,ScopedProperties> expectedProps, boolean ignoreExtra) { | ||
| var scopeNamesInYaml = new HashSet<ScopeName>(); | ||
| allProps.forEach(sp -> scopeNamesInYaml.add(new ScopeName(sp.scope(), sp.name()))); | ||
| allProps.forEach(sp -> { | ||
| if (!scopeNamesInYaml.add(new ScopeName(sp))) { | ||
| throw new IllegalArgumentException( | ||
| "Duplicate scope+name in input, scope:" + sp.scope() + " name:" + sp.name()); | ||
| } | ||
| }); | ||
| var scopeNamesInAccumulo = getAllScopeNames(serverContext); | ||
|
|
||
| if (!scopeNamesInYaml.equals(scopeNamesInAccumulo)) { | ||
|
|
@@ -149,29 +171,85 @@ private static void validate(ServerContext serverContext, List<ScopedProperties> | |
| } | ||
| } | ||
|
|
||
| if (expectedProps != null) { | ||
| for (var scopedProps : allProps) { | ||
| var key = new ScopeName(scopedProps.scope(), scopedProps.name()); | ||
| if (!expectedProps.containsKey(key)) { | ||
| throw new IllegalArgumentException( | ||
| "Scope+name present in input but not present in expected file, scope:" + key.scope() | ||
| + " name:" + key.name()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // validate all scope+name before attempting to update any scope+name | ||
| for (var scopedProps : allProps) { | ||
| var propStoreKey = getKey(scopedProps.scope(), scopedProps.name(), serverContext); | ||
| PropUtil.validateProperties(serverContext, propStoreKey, scopedProps.props()); | ||
| } | ||
| } | ||
|
|
||
| private static List<ScopedProperties> read(Yaml yaml, String file, InputStream in) { | ||
| List<ScopedProperties> allProps = new ArrayList<>(); | ||
| if (file != null) { | ||
| try (var fileIn = new BufferedInputStream(Files.newInputStream(Path.of(file)))) { | ||
| for (var obj : yaml.loadAll(fileIn)) { | ||
| allProps.add(new ScopedProperties((Map<?,?>) obj)); | ||
| } | ||
| } catch (IOException e) { | ||
| throw new UncheckedIOException(e); | ||
| } | ||
| } else { | ||
| for (var obj : yaml.loadAll(in)) { | ||
| allProps.add(new ScopedProperties((Map<?,?>) obj)); | ||
| } | ||
| } | ||
|
|
||
| return allProps; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| public static void load(ServerContext serverContext, InputStream in, Opts options) { | ||
| Yaml yaml = new Yaml(); | ||
| List<ScopedProperties> allProps = new ArrayList<>(); | ||
| for (var obj : yaml.loadAll(in)) { | ||
| allProps.add(new ScopedProperties((Map<?,?>) obj)); | ||
| var loaderOpts = new LoaderOptions(); | ||
| loaderOpts.setAllowDuplicateKeys(false); | ||
| Yaml yaml = new Yaml(loaderOpts); | ||
| List<ScopedProperties> allProps = read(yaml, options.inputFile, in); | ||
|
|
||
| Map<ScopeName,ScopedProperties> expectedProps; | ||
| if (options.expectedFile == null) { | ||
| expectedProps = null; | ||
| } else { | ||
| var grouped = new HashMap<ScopeName,ScopedProperties>(); | ||
| for (var sp : read(yaml, options.expectedFile, null)) { | ||
| var key = new ScopeName(sp); | ||
| if (grouped.put(key, sp) != null) { | ||
| throw new IllegalArgumentException( | ||
| "Duplicate scope+name in expected file, scope:" + sp.scope() + " name:" + sp.name()); | ||
| } | ||
| } | ||
| expectedProps = grouped; | ||
| } | ||
|
|
||
| validate(serverContext, allProps, options.ignoreExtra); | ||
| validate(serverContext, allProps, expectedProps, options.ignoreExtra); | ||
|
|
||
| if (!options.dryRun) { | ||
| var propStore = serverContext.getPropStore(); | ||
|
|
||
| for (var sp : allProps) { | ||
| var propStoreKey = getKey(sp.scope(), sp.name(), serverContext); | ||
| propStore.replaceAll(propStoreKey, sp.props()); | ||
| if (expectedProps == null) { | ||
| // Unconditionally replace properties | ||
| propStore.replaceAll(propStoreKey, sp.props()); | ||
| } else { | ||
| try { | ||
| // Only replace properties if they match the expected values | ||
| propStore.replaceAll(propStoreKey, expectedProps.get(new ScopeName(sp)).props(), | ||
| sp.props()); | ||
| } catch (ConcurrentModificationException cme) { | ||
| throw new ConcurrentModificationException("Properties in scope:" + sp.scope() + " name:" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be useful to either print the current properties or print the difference between the expected and input set.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this case was thinking the user can export config to a new file and use diff on the command line to see what is going on, I will update the error message to suggest that.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made an update in a73c921 |
||
| + sp.name() + " do not match the expected values.", cme); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realized the --dryrun option is not checking expected, need to add that here.