-
Notifications
You must be signed in to change notification settings - Fork 459
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
Fix tempfilter with multi stage deployments #2248
Merged
and-rewsmith
merged 27 commits into
Azure:master
from
and-rewsmith:andsmi/e2e-tempfilter-enumerable
Jan 7, 2020
Merged
Fix tempfilter with multi stage deployments #2248
and-rewsmith
merged 27 commits into
Azure:master
from
and-rewsmith:andsmi/e2e-tempfilter-enumerable
Jan 7, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…otedge into andsmi/new-e2e-tempfilter
…smith/iotedge into andsmi/e2e-tempfilter-enumerable
philipktlin
reviewed
Jan 7, 2020
test/Microsoft.Azure.Devices.Edge.Test.Common/config/EdgeConfigBuilder.cs
Outdated
Show resolved
Hide resolved
philipktlin
reviewed
Jan 7, 2020
test/Microsoft.Azure.Devices.Edge.Test.Common/config/EdgeConfigBuilder.cs
Outdated
Show resolved
Hide resolved
and-rewsmith
commented
Jan 7, 2020
@@ -48,13 +48,18 @@ public async Task<EdgeDeployment> DeployConfigurationAsync(Action<EdgeConfigBuil | |||
addConfig(builder); | |||
|
|||
DateTime deployTime = DateTime.Now; | |||
EdgeConfiguration config = builder.Build(); | |||
IEnumerable<EdgeConfiguration> configs = builder.BuildConfigurationStages(); |
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.
Let me know what you think of this name change
philipktlin
reviewed
Jan 7, 2020
@@ -72,21 +75,25 @@ public EdgeConfiguration Build() | |||
var moduleNames = new List<string>(); | |||
var moduleImages = new List<string>(); | |||
|
|||
foreach (ModuleConfiguration module in modules) | |||
string edgeHubModuleId = "$edgeHub"; |
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.
uses Constants.EdgeHubModuleIdentityName in EdgeAgent.Core project
…smith/iotedge into andsmi/e2e-tempfilter-enumerable
philipktlin
reviewed
Jan 7, 2020
test/Microsoft.Azure.Devices.Edge.Test.Common/config/EdgeConfigBuilder.cs
Show resolved
Hide resolved
philipktlin
approved these changes
Jan 7, 2020
and-rewsmith
changed the title
Fix tempfilter with IEnumerable
Fix tempfilter with multi stage deployments
Jan 7, 2020
kodiakhq bot
pushed a commit
that referenced
this pull request
Feb 22, 2020
The end-to-end tests were recently updated (see #2248 and #2442) to do deployments in two stages: the first just adds the system modules (including any edge hub routes the test will need), and the second adds the remaining modules. This was done to give edge hub a chance to get routes set up before test devices/modules begin using them. Following the changes, output showed that the staged deployments were happening as intended, but they weren't. A deployment config really breaks down into two major parts: edge agent's desired properties (which describe all the modules in the deployment), and all the other modules' desired properties (e.g. edge hub routes, test module config, etc.). The first deployment contained the first part and the second deployment contained the first and second parts, which means that all modules were being sent down to the device in the first deployment. This change fixes the problem by calling the internal function `EdgeConfigBuilder.BuildEdgeAgent` twice--once for each deployment. Each call receives only the modules that are expected to start with that deployment (just system modules first, all modules second). I also added constants for `$edgeAgent` and `$edgeHub`.
kodiakhq bot
pushed a commit
that referenced
this pull request
Jul 27, 2020
Given recent improvements to the end-to-end test framework (#2635, #2753), the logic that splits deployments into multiple stages (#2248) is no longer needed. This change removes that logic. In verifying the changes, I also ran into occasional failures in the `ValidateMetrics` test because the stringized JSON labels that were recently added to Edge Agent containers failed to compare. The expected and actual twin reported properties always had the same JSON properties, but sometimes in different orders: ``` systemModules.edgeAgent.settings.createOptions: {"Labels":{"net.azure-devices.edge.create-options":"{}","net.azure-devices.edge.env":"{\"RuntimeLogLevel\":{\"value\":\"debug\"},\"PerformanceMetricsUpdateFrequency\":{\"value\":\"00:00:20\"}}","net.azure-devices.edge.owner":"Microsoft.Azure.Devices.Edge.Agent"} systemModules.edgeAgent.settings.createOptions: {"Labels":{"net.azure-devices.edge.create-options":"{}","net.azure-devices.edge.env":"{\"PerformanceMetricsUpdateFrequency\":{\"value\":\"00:00:20\"},\"RuntimeLogLevel\":{\"value\":\"debug\"}}","net.azure-devices.edge.owner":"Microsoft.Azure.Devices.Edge.Agent"} ``` The logic that compares these properties is trying to confirm that the configuration has been deployed, and the agent's labels aren't related to the deployment so I just removed them from the comparison.
damonbarry
added a commit
to damonbarry/iotedge
that referenced
this pull request
Aug 5, 2020
Given recent improvements to the end-to-end test framework (Azure#2635, Azure#2753), the logic that splits deployments into multiple stages (Azure#2248) is no longer needed. This change removes that logic. In verifying the changes, I also ran into occasional failures in the `ValidateMetrics` test because the stringized JSON labels that were recently added to Edge Agent containers failed to compare. The expected and actual twin reported properties always had the same JSON properties, but sometimes in different orders: ``` systemModules.edgeAgent.settings.createOptions: {"Labels":{"net.azure-devices.edge.create-options":"{}","net.azure-devices.edge.env":"{\"RuntimeLogLevel\":{\"value\":\"debug\"},\"PerformanceMetricsUpdateFrequency\":{\"value\":\"00:00:20\"}}","net.azure-devices.edge.owner":"Microsoft.Azure.Devices.Edge.Agent"} systemModules.edgeAgent.settings.createOptions: {"Labels":{"net.azure-devices.edge.create-options":"{}","net.azure-devices.edge.env":"{\"PerformanceMetricsUpdateFrequency\":{\"value\":\"00:00:20\"},\"RuntimeLogLevel\":{\"value\":\"debug\"}}","net.azure-devices.edge.owner":"Microsoft.Azure.Devices.Edge.Agent"} ``` The logic that compares these properties is trying to confirm that the configuration has been deployed, and the agent's labels aren't related to the deployment so I just removed them from the comparison.
ggjjj
pushed a commit
to ggjjj/iotedge
that referenced
this pull request
Aug 20, 2020
Given recent improvements to the end-to-end test framework (Azure#2635, Azure#2753), the logic that splits deployments into multiple stages (Azure#2248) is no longer needed. This change removes that logic. In verifying the changes, I also ran into occasional failures in the `ValidateMetrics` test because the stringized JSON labels that were recently added to Edge Agent containers failed to compare. The expected and actual twin reported properties always had the same JSON properties, but sometimes in different orders: ``` systemModules.edgeAgent.settings.createOptions: {"Labels":{"net.azure-devices.edge.create-options":"{}","net.azure-devices.edge.env":"{\"RuntimeLogLevel\":{\"value\":\"debug\"},\"PerformanceMetricsUpdateFrequency\":{\"value\":\"00:00:20\"}}","net.azure-devices.edge.owner":"Microsoft.Azure.Devices.Edge.Agent"} systemModules.edgeAgent.settings.createOptions: {"Labels":{"net.azure-devices.edge.create-options":"{}","net.azure-devices.edge.env":"{\"PerformanceMetricsUpdateFrequency\":{\"value\":\"00:00:20\"},\"RuntimeLogLevel\":{\"value\":\"debug\"}}","net.azure-devices.edge.owner":"Microsoft.Azure.Devices.Edge.Agent"} ``` The logic that compares these properties is trying to confirm that the configuration has been deployed, and the agent's labels aren't related to the deployment so I just removed them from the comparison.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TempFilter tests sometimes fail due to the modules starting before the necessary edgeHub routes are in place. I made a fix for this where the new e2e framework deploys in configurations. First it deploys edgeAgent / edgeHub, then the rest of the modules.