CheckHealth.ps1 bug

[scholtz]

Hello,

file https://github.com/microsoft/nav-docker/blob/main/generic/Run/CheckHealth.ps1 contains a bug that if custom $publicWebBaseUrl is configured, it checks for the health on public available endpoint. however if one runs the health checks in k8s in multiple instances, one pod would get information that it is healthy because other pod sends this information.

health check should not be done on public url, but inside the container

in the single pod instance where startup probe checks this endpoint, it will never start as the startup probe is never ready on the public url because the pod is not in state ready to allow public network routing

[mazhelez]

The health check is an "outside-in" check, so it's supposed to indeed check the public URL.

I don't quite get your setup, can you elaborate more?

[scholtz]

@mazhelez thanks for reply

we are using on prem business central

we have 4 environments dev,test,pp,prod. dev and test runs in k8s. pp and prod not yet because msft has warning in this repo that docker is not yet decided if it will be supported, but we would very much like to run it this way as it has many features like no outages while being restarted, HA support, scalability, liveness monitoring and more..

we built docker image using new bc image and store it to our docker registry

Import-Module BcContainerHelper
# Update-Module -Name BcContainerHelper -Force

$image = "myimage:onprem-26.3.36158.36321-cz-nodb"
$extensionsFolder = "aricoma/26.3.36158.0";
$licenseFilePath = 'C:\BC\licencie\licence.bclicense'
$art = Get-BCArtifactUrl -country "cz" -version "26" -type OnPrem

$art

# #exit

New-BcImage -artifactUrl $art -Verbose:$true -skipDatabase -includeTestToolkit:$false -includeTestLibrariesOnly:$false -licenseFile $licenseFilePath

$dockerfilePath = "create-image-26.dockerfile"

$fileContent = Get-Content -Path $dockerfilePath

# Replace the first line starting with "FROM myimage:" with the new value
$newContent = $fileContent
$newContent[0] = "FROM $image"

# Save the updated content back to the file
Set-Content -Path $dockerfilePath -Value $newContent

Write-Output "Dockerfile updated successfully."

$bcimage = $image -replace "^myimage", "bc"
docker build -f create-image-26.dockerfile -t czphatestk8sreg01.aaa.cz/$bcimage $extensionsFolder
docker push czphatestk8sreg01.aaa.cz/$bcimage
Write-Output "czphatestk8sreg01.aaa.cz/$bcimage"

docker file

FROM myimage:onprem-26.3.36158.36321-cz-nodb
ENV DatabaseInstance=

ARG src1=".netpackages"
ARG target1="c:\\Program Files\\Microsoft Dynamics NAV\\260\\Service\\Add-ins\\"

COPY ${src1} ${target1}
ARG src2="*.app"
ARG target2="c:\\aricoma-extensions\\"
COPY ${src2} ${target2}

ARG src3="install-aricoma-26.ps1"
ARG target3="c:\\aricoma-extensions\\install.ps1"
COPY ${src3} ${target3}

ARG src3="SyncUsersLdap2ERP"
ARG target3="c:\\AAA\\SyncUsersLdap2ERP"
COPY ${src3} ${target3}

we are using helm chart https://artifacthub.io/packages/helm/lippertmarkus/business-central to deploy the business central

in k8s the helm chart is using the health check for probe if the pod is in ready state, but actually it checks if the whole deployment is working

          livenessProbe:
            exec:
              command:
                - powershell
                - C:\Run\CheckHealth.ps1
            failureThreshold: 5
            periodSeconds: 5
            successThreshold: 1
            timeoutSeconds: 1
          readinessProbe:
            exec:
              command:
                - powershell
                - C:\Run\CheckHealth.ps1
            failureThreshold: 5
            periodSeconds: 5
            successThreshold: 1
            timeoutSeconds: 1
          startupProbe:
            exec:
              command:
                - powershell
                - C:\Run\CheckHealth.ps1
            failureThreshold: 60
            initialDelaySeconds: 600
            periodSeconds: 10
            successThreshold: 1
            timeoutSeconds: 2

it does not make much sense for me to add to docker image the health check if the outside cluster is running.. when we did not modified the public url it worked fine because it was checking the local docker container if it works, but after we changed the public url it stopped working.

we need to modify the public url because we publish our business central at bc.dev.our.domain, and some links in business central does not work because they are rendered with the hostname as it runs in the pod

the solution for us it to create custom health check file and use that, but it would be much nicer if the issue was fixed at the source so that others do not hit the same issue

[scholtz]

if there would be configurable url from env variable to check the health, something like in https://github.com/microsoft/nav-docker/pull/456/files , it would be great while preserving the old functionality.. but someone should think it through what sense it makes to do health check on cluster it self and not the docker container it self

[tfenster]

I deeply admire your self-confidence to say "this is stupid" for a solution that is in place and used successfully for years

[freddykristiansen]

If you are using New-BcContainer to create a container - it is easier to add -doNotCheckHealth to your command than writing many lines describing that you don't want health check.
If you are not using New-BcContainer, then add --no-healthcheck to your docker exec and you are home safe.
If you want health check - just different - then override the healthcheck script and you are home safe.

Please note that if you are using docker containers to run customers in production, you are expected to be able to investigate and make those changes yourself - Microsoft doesn't have an SLA on what and when bugs in the generic image are fixed.

Containers are primarily designed for dev/test purposes - maybe that is also stupid - but we have the cloud for production environment...

[scholtz]

I deeply admire your self-confidence to say "this is stupid" for a solution that is in place and used successfully for years

i did not use word stupid, but can you please provide a reasoning why the health check is checking the outer functionality and not inner state of the container?

If you are using New-BcContainer to create a container - it is easier to add -doNotCheckHealth to your command than writing many lines describing that you don't want health check. If you are not using New-BcContainer, then add --no-healthcheck to your docker exec and you are home safe. If you want health check - just different - then override the healthcheck script and you are home safe.
I absolutely do not want "not to use health checks" .. it is the primary functionality of k8s to provide the liveness functionality in kubernetes to determine if the container should be arranged in different worker node for example, and helps to switch the networking when the bc restarts so that it is without outage

i am highlighting the issue in this repo and hope that more people find it frustrating it does not work as it should be

i understand that we can modify the file by our selves, but wanted to fix it at the root as others may have the same issue and i do not understand why it should work like that.. and pr 456 indicates that not only we have this issue and the issue is here for years now

[freddykristiansen]

I cannot remember why the outside-in approach was made back in the day, I think it was to ensure that if you are publishing the port on the host or having an SSL certificate - then everything was tested through this simple mechanism, knowing however that if you are running behind a load balancer or other things - it would have to be different, but the people who are indeed doing this, also has the knowledge to add a custom health check for testing their scenario.

[scholtz]

How do you recommend to proceed?

A - Creating the correct health check and moving existing to different name. This would in time solve the helm and k8s deployments automatically for users.

B - Creating correct health check with different name, and recomending to use the new health check as it is safer, afterwads fixing all helm repos

C - Do nothing and keep new users who wants to run the BC in k8s waste their time on this

?

[Freddy-D4P]

I think (note that I am no longer the owner), you should create a PR, which doesn't change the existing functionality unless either specifically specified or if you can safely discover you are running in k8s,

I will happily review and hope that MS will as well.

Just noticed that this is written in another one of my profiles