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

Support local deployment's param version matching #404

Merged
merged 5 commits into from
Sep 1, 2020
Merged

Support local deployment's param version matching #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a2dc0a
Support local deployment's param version matching
leaf94 Aug 31, 2020
6e7ec30
style
leaf94 Aug 31, 2020
713e14f
Fix test due to a potential semver bug
leaf94 Sep 1, 2020
e552aed
Get back tests
leaf94 Sep 1, 2020
a4170b1
Merge branch 'master' into local_dev_param_version_match
leaf94 Sep 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/main/java/com/aws/iot/evergreen/packagemanager/KernelConfigResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import com.aws.iot.evergreen.util.CrashableFunction;
import com.aws.iot.evergreen.util.Pair;
import com.aws.iot.evergreen.util.Utils;
import com.vdurmont.semver4j.Requirement;
import com.vdurmont.semver4j.Semver;

import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -295,8 +297,12 @@ private Optional<DeploymentPackageConfiguration> getMatchingPackageConfigFromDep
String packageName,
String packageVersion) {
return document.getDeploymentPackageConfigurationList().stream()
.filter(packageConfig -> packageName.equals(packageConfig.getPackageName()) && packageVersion
.equals(packageConfig.getResolvedVersion())).findAny();
.filter(packageConfig ->
packageName.equals(packageConfig.getPackageName())
// TODO packageConfig.getResolvedVersion() should be strongly typed when created
&& Requirement.buildNPM(packageConfig.getResolvedVersion())
.isSatisfiedBy(new Semver(packageVersion, Semver.SemverType.NPM)))
.findAny();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is findAny() still correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. findAny and findFirst might both be correct, as a deployment doc could have multiple entries that satisfy the resolved version (which might change). Let's keep it for now I guess.

}

private Set<PackageParameter> resolveParameterValuesToUseWithCache(
Expand Down
25 changes: 12 additions & 13 deletions src/test/java/com/aws/iot/evergreen/packagemanager/KernelConfigResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.aws.iot.evergreen.deployment.model.DeploymentPackageConfiguration;
import com.aws.iot.evergreen.kernel.EvergreenService;
import com.aws.iot.evergreen.kernel.Kernel;
import com.aws.iot.evergreen.packagemanager.exceptions.PackageLoadingException;
import com.aws.iot.evergreen.packagemanager.models.PackageIdentifier;
import com.aws.iot.evergreen.packagemanager.models.PackageParameter;
import com.aws.iot.evergreen.packagemanager.models.PackageRecipe;
Expand Down Expand Up @@ -111,9 +110,9 @@ void GIVEN_deployment_for_package_WHEN_config_resolution_requested_THEN_add_serv
TEST_INPUT_PACKAGE_B);

DeploymentPackageConfiguration rootPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "1.2", Collections.emptyMap());
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "=1.2", Collections.emptyMap());
DeploymentPackageConfiguration dependencyPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_B, false, "2.3", Collections.emptyMap());
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_B, false, "=2.3", Collections.emptyMap());
DeploymentDocument document = DeploymentDocument.builder()
.rootPackages(Arrays.asList(TEST_INPUT_PACKAGE_A))
.deploymentPackageConfigurationList(
Expand Down Expand Up @@ -149,7 +148,6 @@ void GIVEN_deployment_for_package_WHEN_config_resolution_requested_THEN_add_serv
dependencyListContains("main", TEST_INPUT_PACKAGE_A, servicesConfig));
assertThat("Main service must depend on existing service",
dependencyListContains("main", "IpcService" + ":" + DependencyType.HARD, servicesConfig));
System.out.println(servicesConfig);
assertThat("New service must depend on dependency service",
dependencyListContains(TEST_INPUT_PACKAGE_A, TEST_INPUT_PACKAGE_B, servicesConfig));

Expand All @@ -167,7 +165,7 @@ void GIVEN_deployment_for_existing_package_WHEN_config_resolution_requested_THEN
TEST_INPUT_PACKAGE_A);

DeploymentPackageConfiguration rootPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "1.2", Collections.emptyMap());
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "=1.2", Collections.emptyMap());
DeploymentDocument document = DeploymentDocument.builder()
.rootPackages(Arrays.asList(TEST_INPUT_PACKAGE_A))
.deploymentPackageConfigurationList(
Expand Down Expand Up @@ -213,7 +211,7 @@ void GIVEN_deployment_with_parameters_set_WHEN_config_resolution_requested_THEN_
getSimpleParameterMap(TEST_INPUT_PACKAGE_A), TEST_INPUT_PACKAGE_A);

DeploymentPackageConfiguration rootPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "1.2", new HashMap<String, Object>() {{
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, ">=1.2", new HashMap<String, Object>() {{
put("PackageA_Param_1", "PackageA_Param_1_value");
}});
DeploymentDocument document = DeploymentDocument.builder()
Expand Down Expand Up @@ -273,22 +271,24 @@ void GIVEN_deployment_with_parameters_set_WHEN_config_resolution_requested_THEN_

PackageRecipe rootPackageRecipe = getPackage(TEST_INPUT_PACKAGE_A, "1.2", Collections.emptyMap(),
getSimpleParameterMap(TEST_INPUT_PACKAGE_A), TEST_INPUT_PACKAGE_A);

// B-1.5 -> A-1.2
PackageRecipe package2Recipe = getPackage(TEST_INPUT_PACKAGE_B, "1.5", Utils.immutableMap(TEST_INPUT_PACKAGE_A,
new RecipeDependencyProperties("=1.2", DependencyType.HARD.toString())),
getSimpleParameterMap(TEST_INPUT_PACKAGE_B), TEST_INPUT_PACKAGE_A);
PackageRecipe package3Recipe = getPackage(TEST_INPUT_PACKAGE_C, "1.5", Collections.emptyMap(),
getSimpleParameterMap(TEST_INPUT_PACKAGE_C), TEST_INPUT_PACKAGE_A);

DeploymentPackageConfiguration rootPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "1.2", new HashMap<String, Object>() {{
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "=1.2", new HashMap<String, Object>() {{
put("PackageA_Param_1", "PackageA_Param_1_value");
}});
DeploymentPackageConfiguration package2DeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_B, true, "1.2", new HashMap<String, Object>() {{
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_B, true, "=1.5", new HashMap<String, Object>() {{
put("PackageB_Param_1", "PackageB_Param_1_value");
}});
DeploymentPackageConfiguration package3DeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_C, true, "1.2", Collections.emptyMap());
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_C, true, "=1.5", Collections.emptyMap());
DeploymentDocument document = DeploymentDocument.builder()
.rootPackages(Arrays.asList(TEST_INPUT_PACKAGE_A,
TEST_INPUT_PACKAGE_B, TEST_INPUT_PACKAGE_C))
Expand Down Expand Up @@ -342,7 +342,7 @@ void GIVEN_deployment_with_params_not_set_WHEN_previous_deployment_had_params_TH
getSimpleParameterMap(TEST_INPUT_PACKAGE_A), TEST_INPUT_PACKAGE_A);

DeploymentPackageConfiguration rootPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "1.2", Collections.emptyMap());
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "=1.2", Collections.emptyMap());
DeploymentDocument document = DeploymentDocument.builder()
.rootPackages(Arrays.asList(TEST_INPUT_PACKAGE_A))
.deploymentPackageConfigurationList(
Expand Down Expand Up @@ -406,7 +406,7 @@ void GIVEN_deployment_with_artifact_WHEN_config_resolution_requested_THEN_artifa
}}, Collections.emptyList(), Collections.emptyMap(), null);

DeploymentPackageConfiguration rootPackageDeploymentConfig =
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "1.2", Collections.emptyMap());
new DeploymentPackageConfiguration(TEST_INPUT_PACKAGE_A, true, "=1.2", Collections.emptyMap());
Copy link
Member

Choose a reason for hiding this comment

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

What if the customer isn't specifying requirements? What if it is just 1.2?

Copy link
Member

Choose a reason for hiding this comment

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

If it needs to be a version range, then we need to make this clear in the cloud

Copy link
Contributor Author

@leaf94 leaf94 Sep 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

1.2 should also work but it doesn't now. But 1.2.0 works. Took me some time to debug and found a werid error from Semver lib... I opened an issue with them: vdurmont/semver4j#46

But in reality, right now cloud and local will send a valid "x.y.z" with patch version specified, so we should be good.

DeploymentDocument document = DeploymentDocument.builder()
.rootPackages(Arrays.asList(TEST_INPUT_PACKAGE_A))
.deploymentPackageConfigurationList(
Expand Down Expand Up @@ -437,8 +437,7 @@ void GIVEN_deployment_with_artifact_WHEN_config_resolution_requested_THEN_artifa
// utilities for mocking input
private PackageRecipe getPackage(String packageName, String packageVersion,
Map<String, RecipeDependencyProperties> dependencies,
Map<String, String> packageParamsWithDefaultsRaw, String crossComponentName)
throws PackageLoadingException {
Map<String, String> packageParamsWithDefaultsRaw, String crossComponentName) {

Set<PackageParameter> parameters = packageParamsWithDefaultsRaw.entrySet()
.stream()
Expand Down