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

ARM template broken due to breaking change in ServiceFabricNodeBootstrapAgent #197

Closed
amolenk opened this Issue Mar 15, 2017 · 2 comments

Comments

Projects
None yet
6 participants
@amolenk
Copy link

amolenk commented Mar 15, 2017

It seems like we’re encountering a breaking change in the ServiceFabricNodeBootstrapAgent. Our ARM script to setup a Service Fabric cluster used to work fine with agent version 1.0.0.139. However, if we do a new deployment now, we get version 1.0.0.143 of the agent which logs the following error in the event log:

Failed starting service, Error: System.ArgumentNullException: Value cannot be null.

I’ve looked at the code and believe I’ve found a breaking change concerning the dataPath setting (part of the ServiceFabricNode extension configuration in the ARM template).

In version 1.0.0.139 of the ServiceFabricNodeBootstrapAgent, the code checks whether the dataPath setting is not null (dataRoot in the code):

if (!string.IsNullOrWhiteSpace(dataRoot))
    {
        Trace.TraceInformation("Using user-specified data root: {0}", (object) dataRoot);
        parameters.Add("FabricDataRoot", (object) dataRoot);
        parameters.Add("FabricLogRoot", (object) Path.Combine(dataRoot, "Log"));
    }

However, in version 1.0.0.143, this check is gone:

num = this.ConfigureNode(str1, str2, Path.GetFullPath(dataRoot), Path.GetFullPath(Path.Combine(dataRoot, "Log")));

In this version, dataRoot is directly passed to the Path.GetFullPath method which then throws the ArgumentNullException.

As a workaround, I guess we can add the dataPath settings to the ARM template. But in my opinion, it would be better to keep the null check in the code.

@abhaymhatre

This comment has been minimized.

Copy link

abhaymhatre commented Mar 15, 2017

Thanks for reporting this. Will make a fix in next iteration.
for mitigation please pass data path in arm template under vm extension as you described

here is an example

{
                "name": "[concat('ServiceFabricNodeVmExt','_vmNodeType0Name')]",
                "properties": {
                  "type": "ServiceFabricNode",
                  "autoUpgradeMinorVersion": false,
                  "protectedSettings": {
                    "StorageAccountKey1": "[listKeys(resourceId('Microsoft.Storage/storageAccounts', variables('supportLogStorageAccountName')),'2015-05-01-preview').key1]",
                    "StorageAccountKey2": "[listKeys(resourceId('Microsoft.Storage/storageAccounts', variables('supportLogStorageAccountName')),'2015-05-01-preview').key2]"
                  },
                  "publisher": "Microsoft.Azure.ServiceFabric.Test",
                  "settings": {
                    "clusterEndpoint": "[reference(parameters('clusterName')).clusterEndpoint]",
                    "nodeTypeRef": "[variables('vmNodeType0Name')]",
                    "dataPath": "D:\\\\SvcFab",
                    "durabilityLevel": "Bronze",
                    "testExtension":  true
                  },
                  "typeHandlerVersion": "1.0"
                }
              }

@masnider masnider added the bug label Mar 20, 2017

@ChackDan ChackDan removed their assignment Sep 8, 2017

@mhatreabhay

This comment has been minimized.

Copy link

mhatreabhay commented Oct 11, 2017

Issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment