Skip to content
This repository has been archived by the owner on Nov 20, 2023. It is now read-only.

adding health checks #432

Merged
merged 35 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7e6c2ef
each replica has its own cancellation token and records its own state
areller Apr 30, 2020
8b3e89c
added state to v1replica
areller Apr 30, 2020
57dc3f4
license and formatting
areller Apr 30, 2020
616df58
add liveness and readiness to configuration and model
areller May 3, 2020
3912db6
ReplicaMonitor
areller May 3, 2020
24b0459
revamp test helpers
areller May 3, 2020
d4fbdeb
health check tests
areller May 3, 2020
e95a361
formatting
areller May 3, 2020
56b9869
merge with master
areller May 3, 2020
9dcc606
formatting
areller May 3, 2020
3f48f54
don't run docker tests
areller May 3, 2020
3b058b3
add more tests
areller May 4, 2020
4659b51
fix header handling
areller May 4, 2020
ee9b374
start adding timeout to http probe
areller May 4, 2020
8f396c7
add timeout, port, protocol, successThreshold, failureThreshold to pr…
areller May 5, 2020
0276a09
success threshold must be 1 for liveness
areller May 7, 2020
569df2a
proxy and ingress don't forward to non ready replicas
areller May 7, 2020
f22149c
warnings and formatting
areller May 7, 2020
f452feb
Merge remote-tracking branch 'dotnet/master' into health
areller May 8, 2020
aa52cd0
fix mistake done by merge
areller May 8, 2020
22e506e
formatting
areller May 8, 2020
7a72a3e
Merge branch 'master' into health
areller May 10, 2020
440cf1b
Merge remote-tracking branch 'dotnet/master' into health
areller May 12, 2020
e86cda1
run proxy for single replica services if readiness is defined
areller May 12, 2020
b41ccff
license and formatting
areller May 12, 2020
1d3d03e
should start at (false, false)
areller May 12, 2020
f89e630
clean csproj
areller May 13, 2020
726e2a4
rename service names in health-checks folder
areller May 13, 2020
7a29291
change name of service in tests
areller May 13, 2020
65c7ae0
generate liveness and readiness in k8s manifest
areller May 13, 2020
ffef174
fix review comments
areller May 13, 2020
6280485
using docker restart when docker replica is cancelled
areller May 14, 2020
a116b4b
formatting
areller May 14, 2020
eb13a21
Update src/Microsoft.Tye.Core/CoreStrings.resx
areller May 18, 2020
602a2bc
Update src/Microsoft.Tye.Core/KubernetesManifestGenerator.cs
areller May 18, 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
27 changes: 27 additions & 0 deletions src/Microsoft.Tye.Core/ApplicationFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ public static async Task<ApplicationBuilder> CreateAsync(OutputContext output, F
}
project.Replicas = configService.Replicas ?? 1;

project.Liveness = configService.Liveness != null ? GetProbeBuilder(configService.Liveness) : null;
project.Readiness = configService.Readiness != null ? GetProbeBuilder(configService.Readiness) : null;

// We don't apply more container defaults here because we might need
// to prompt for the registry name.
project.ContainerInfo = new ContainerInfo() { UseMultiphaseDockerfile = false, };
Expand All @@ -115,6 +118,9 @@ public static async Task<ApplicationBuilder> CreateAsync(OutputContext output, F
DockerFileContext = configService.DockerFileContext != null ? Path.Combine(source.DirectoryName, configService.DockerFileContext) : null
};
service = container;

container.Liveness = configService.Liveness != null ? GetProbeBuilder(configService.Liveness) : null;
container.Readiness = configService.Readiness != null ? GetProbeBuilder(configService.Readiness) : null;
}
else if (!string.IsNullOrEmpty(configService.Executable))
{
Expand All @@ -137,6 +143,9 @@ public static async Task<ApplicationBuilder> CreateAsync(OutputContext output, F
Replicas = configService.Replicas ?? 1
};
service = executable;

executable.Liveness = configService.Liveness != null ? GetProbeBuilder(configService.Liveness) : null;
executable.Readiness = configService.Readiness != null ? GetProbeBuilder(configService.Readiness) : null;
}
else if (!string.IsNullOrEmpty(configService.Include))
{
Expand Down Expand Up @@ -351,5 +360,23 @@ private static void AddToRootServices(ApplicationBuilder root, HashSet<string> d
}
}
}

private static ProbeBuilder GetProbeBuilder(ConfigProbe config) => new ProbeBuilder()
{
Http = config.Http != null ? GetHttpProberBuilder(config.Http) : null,
InitialDelay = config.InitialDelay,
Period = config.Period,
Timeout = config.Timeout,
SuccessThreshold = config.SuccessThreshold,
FailureThreshold = config.FailureThreshold
};

private static HttpProberBuilder GetHttpProberBuilder(ConfigHttpProber config) => new HttpProberBuilder()
{
Path = config.Path,
Headers = config.Headers,
Port = config.Port,
Protocol = config.Protocol
};
}
}
15 changes: 15 additions & 0 deletions src/Microsoft.Tye.Core/ConfigModel/ConfigApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,21 @@ public void Validate()
string.Join(Environment.NewLine, results.Select(r => r.ErrorMessage)));
}
}

var probes = new[] { (Name: "liveness", Probe: service.Liveness), (Name: "readiness", Probe: service.Readiness) }.Where(p => p.Probe != null).ToArray();
foreach (var probe in probes)
{
if (probe.Name == "liveness" && probe.Probe.SuccessThreshold != 1)
{
throw new TyeYamlException(CoreStrings.FormatSuccessThresholdMustBeOne(probe.Name));
}

// right now only http is supported, so it must be set
if (probe.Probe!.Http is null)
{
throw new TyeYamlException(CoreStrings.FormatProberRequired(probe.Name));
}
}
}

foreach (var ingress in config.Ingress)
Expand Down
18 changes: 18 additions & 0 deletions src/Microsoft.Tye.Core/ConfigModel/ConfigHttpProber.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Net.Http.Headers;

namespace Microsoft.Tye.ConfigModel
{
public class ConfigHttpProber
{
[Required] public string Path { get; set; } = default!;
public int? Port { get; set; }
public string? Protocol { get; set; }
public List<KeyValuePair<string, object>> Headers { get; set; } = new List<KeyValuePair<string, object>>();
}
}
16 changes: 16 additions & 0 deletions src/Microsoft.Tye.Core/ConfigModel/ConfigProbe.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.Tye.ConfigModel
{
public class ConfigProbe
{
public ConfigHttpProber? Http { get; set; }
public int InitialDelay { get; set; } = 0;
public int Period { get; set; } = 1;
public int Timeout { get; set; } = 1;
public int SuccessThreshold { get; set; } = 1;
public int FailureThreshold { get; set; } = 3;
}
}
2 changes: 2 additions & 0 deletions src/Microsoft.Tye.Core/ConfigModel/ConfigService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,7 @@ public class ConfigService
[YamlMember(Alias = "env")]
public List<ConfigConfigurationSource> Configuration { get; set; } = new List<ConfigConfigurationSource>();
public List<BuildProperty> BuildProperties { get; set; } = new List<BuildProperty>();
public ConfigProbe? Liveness { get; set; }
public ConfigProbe? Readiness { get; set; }
}
}
3 changes: 3 additions & 0 deletions src/Microsoft.Tye.Core/ContainerServiceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ public ContainerServiceBuilder(string name, string image)
public List<EnvironmentVariableBuilder> EnvironmentVariables { get; } = new List<EnvironmentVariableBuilder>();

public List<VolumeBuilder> Volumes { get; } = new List<VolumeBuilder>();

public ProbeBuilder? Liveness { get; set; }
public ProbeBuilder? Readiness { get; set; }
jkotalik marked this conversation as resolved.
Show resolved Hide resolved
}
}
9 changes: 9 additions & 0 deletions src/Microsoft.Tye.Core/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@
<data name="MultipleBindingWithSamePort" xml:space="preserve">
<value>Cannot have multiple {0} bindings with the same port.</value>
</data>
<data name="ProberRequired" xml:space="preserve">
<value>A prober must be configured for the {0} probe.</value>
</data>
<data name="SuccessThresholdMustBeOne" xml:space="preserve">
<value>successThreshold for {0} probe must be one.</value>
areller marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="MustBeABoolean" xml:space="preserve">
<value>"{value}" must be a boolean value (true/false).</value>
</data>
Expand All @@ -150,6 +156,9 @@
<data name="MustBePositive" xml:space="preserve">
<value>"{value}" value cannot be negative.</value>
</data>
<data name="MustBeGreaterThanZero" xml:space="preserve">
<value>"{value}" value must be greater than zero.</value>
</data>
<data name="ProjectImageExecutableExclusive" xml:space="preserve">
<value>Cannot have both "{0}" and "{1}" set for a service. Only one of project, image, and executable can be set for a given service.</value>
</data>
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.Tye.Core/ExecutableServiceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,8 @@ public ExecutableServiceBuilder(string name, string executable)
public int Replicas { get; set; } = 1;

public List<EnvironmentVariableBuilder> EnvironmentVariables { get; } = new List<EnvironmentVariableBuilder>();

public ProbeBuilder? Liveness { get; set; }
jkotalik marked this conversation as resolved.
Show resolved Hide resolved
public ProbeBuilder? Readiness { get; set; }
}
}
16 changes: 16 additions & 0 deletions src/Microsoft.Tye.Core/HttpProberBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;

namespace Microsoft.Tye
{
public class HttpProberBuilder
{
public string Path { get; set; } = default!;
public int? Port { get; set; }
public string? Protocol { get; set; }
public List<KeyValuePair<string, object>> Headers { get; set; } = new List<KeyValuePair<string, object>>();
}
}
68 changes: 68 additions & 0 deletions src/Microsoft.Tye.Core/KubernetesManifestGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,16 @@ public static KubernetesDeploymentOutput CreateDeployment(
}
}
}

if (project.Liveness != null)
{
AddProbe(project, container, project.Liveness!, "livenessProbe");
}

if (project.Readiness != null)
{
AddProbe(project, container, project.Readiness!, "readinessProbe");
}
}

foreach (var sidecar in project.Sidecars)
Expand Down Expand Up @@ -452,6 +462,64 @@ public static KubernetesDeploymentOutput CreateDeployment(
return new KubernetesDeploymentOutput(project.Name, new YamlDocument(root));
}

private static void AddProbe(ServiceBuilder service, YamlMappingNode container, ProbeBuilder builder, string name)
{
var probe = new YamlMappingNode();
container.Add(name, probe);

if (builder.Http != null)
{
var builderHttp = builder.Http!;
var httpGet = new YamlMappingNode();

probe.Add("httpGet", httpGet);
httpGet.Add("path", builderHttp.Path);

if (builderHttp.Protocol != null)
{
httpGet.Add("scheme", builderHttp.Protocol!.ToUpper());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you don't need ! here, other similar cases in this file as well.

Suggested change
httpGet.Add("scheme", builderHttp.Protocol!.ToUpper());
httpGet.Add("scheme", builderHttp.Protocol.ToUpper());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep. I removed it and it didn't throw any warnings. probably because it's inside a block that runs if builderHttp.Protocol != null.
It's weird because at times the compiler will throw a warning when not using the ! operator, even though the expression is in a context where it's guaranteed to not be null...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

found an example: if you go to ReplicaMonitor.cs:113 and remove the ! after serviceDesc.Readiness, the compiler will throw a warning, even though there is no way for that variable to be null at that point.

}

if (builderHttp.Port != null)
{
httpGet.Add("port", builderHttp.Port!.ToString()!);
}
else
{
// If port is not given, we pull default port
var binding = service.Bindings.First(b => builderHttp.Protocol is null || b.Protocol == builderHttp.Protocol);
Copy link
Contributor

Choose a reason for hiding this comment

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

It does feel a bit awkward to have health checks per service rather than per endpoint, but it seems to be what k8s does. Or maybe we require a port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

requiring a port will indeed make it more compatible with k8s, but it will make the tye.yaml more verbose and less maintainable in my opinion. one thing that k8s does to help with maintainability is to allow to specify a variable as a port. e.g.

port: liveness-port

if (binding.Port != null)
{
httpGet.Add("port", binding.Port.Value.ToString());
}

if (builderHttp.Protocol is null && binding.Protocol != null)
{
httpGet.Add("scheme", binding.Protocol.ToUpper());
}
}

if (builderHttp.Headers.Count > 0)
{
var headers = new YamlSequenceNode();
httpGet.Add("httpHeaders", headers);

foreach (var builderHeader in builderHttp.Headers)
{
var header = new YamlMappingNode();
header.Add("name", builderHeader.Key);
header.Add("value", builderHeader.Value.ToString()!);
headers.Add(header);
}
}
}

probe.Add("initialDelaySeconds", builder.InitialDelay.ToString()!);
areller marked this conversation as resolved.
Show resolved Hide resolved
probe.Add("periodSeconds", builder.Period.ToString()!);
probe.Add("successThreshold", builder.SuccessThreshold.ToString()!);
probe.Add("failureThreshold", builder.FailureThreshold.ToString()!);
}

private static void AddEnvironmentVariablesForComputedBindings(YamlSequenceNode env, ComputedBindings bindings)
{
foreach (var binding in bindings.Bindings.OfType<EnvironmentVariableInputBinding>())
Expand Down
16 changes: 16 additions & 0 deletions src/Microsoft.Tye.Core/ProbeBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.Tye
{
public class ProbeBuilder
{
public HttpProberBuilder? Http { get; set; }
public int InitialDelay { get; set; }
public int Period { get; set; }
public int Timeout { get; set; }
public int SuccessThreshold { get; set; }
public int FailureThreshold { get; set; }
}
}
3 changes: 3 additions & 0 deletions src/Microsoft.Tye.Core/ProjectServiceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,8 @@ public ProjectServiceBuilder(string name, FileInfo projectFile)
public Dictionary<string, string> BuildProperties { get; } = new Dictionary<string, string>();

public List<SidecarBuilder> Sidecars { get; } = new List<SidecarBuilder>();

public ProbeBuilder? Liveness { get; set; }
public ProbeBuilder? Readiness { get; set; }
jkotalik marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading