fix: custom runtime container name is invalid

[lcgash]

Description

fix executor container name is error when runtime container custom name， like this

Fixes #

Testing

I Have Tested This Fix In My Production Env.

Checklist:

• [ √ ] I ran tests as well as code linting locally to verify my changes.
• [√ ] I have done manual verification of my changes, changes working as expected.
• I have added new tests to cover my changes.
• My changes follow contributing guidelines of Fission.
• [√ ] I have signed all of my commits.

[lcgash]

@soharab-ic

[soharab-ic]

@lcgash
Is there any specific reason why you want to use custom name for runtime container? If so then please raise an issue.

There are certain assumption that we follow. It makes merging the specs easier while creating the deployment spec for poolmgr and deploy executor types:

• Runtime container name should be equal to environment name.
• Builder container name should be builder.

You can refer to this PR #3033 for incoming changes (better readability) to environment spec. This will be available in next release.

[lcgash]

@lcgash Is there any specific reason why you want to use custom name for runtime container? If so then please raise an issue.

There are certain assumption that we follow. It makes merging the specs easier while creating the deployment spec for poolmgr and deploy executor types:

• Runtime container name should be equal to environment name.
• Builder container name should be builder.

You can refer to this PR #3033 for incoming changes (better readability) to environment spec. This will be available in next release.

Yes, in our actual production environment, we have already set up a complex container ecosystem and cluster. Metrics, logs, and other systems are standardized to display and collect using the main container. For us, an unknown primary container name like go-xx or node-xx is meaningless and difficult to capture. We noticed that there is such a field defined but not used, so we made this change. This change has now been implemented in our production cluster.

[sanketsudake]

@Mergifyio rebase

[mergify]

rebase

✅ Branch has been successfully rebased

[codecov]

Codecov Report

Attention: Patch coverage is 64.28571% with 10 lines in your changes missing coverage. Please review.

Project coverage is 44.54%. Comparing base (54b67b5) to head (1a6c294).
Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/executor/executortype/newdeploy/newdeploy.go	66.66%	2 Missing and 1 partial ⚠️
pkg/executor/executortype/poolmgr/gp_deployment.go	66.66%	2 Missing and 1 partial ⚠️
pkg/fission-cli/cmd/spec/spec.go	25.00%	2 Missing and 1 partial ⚠️
pkg/executor/util/util.go	83.33%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3065   +/-   ##
=======================================
  Coverage   44.53%   44.54%           
=======================================
  Files         236      237    +1     
  Lines       24735    24833   +98     
=======================================
+ Hits        11016    11061   +45     
- Misses      12313    12347   +34     
- Partials     1406     1425   +19     

Flag	Coverage Δ
unittests	44.54% <64.28%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sanketsudake]

Need to verify if fix can cause any issue for the rolling update to Fission users.
This should not break already created specs with Fission CLI by fission users.

[lcgash]

Need to verify if fix can cause any issue for the rolling update to Fission users. This should not break already created specs with Fission CLI by fission users.

agree。But i do not know how to test it， this change is breaking change 「if users custom runtime container name in spec」。

[sanketsudake]

We need to a check if podspec has a container with similar name. This way we can ensure there are no duplicate entries when container and podspec are merged.

[soharab-ic]

@lcgash Please apply this patch and do the following testing.

• Install fission version v120.5

export FISSION_NAMESPACE="fission"
kubectl create namespace $FISSION_NAMESPACE
kubectl create -k "github.com/fission/fission/crds/v1?ref=v1.20.5"
helm repo add fission-charts https://fission.github.io/fission-charts/
helm repo update
helm install --version v1.20.5 --namespace $FISSION_NAMESPACE fission \
  --set serviceType=NodePort,routerServiceType=NodePort \
  fission-charts/fission-all

• Create specs:

fission spec init
fission env create --name go --image ghcr.io/fission/go-env --builder ghcr.io/fission/go-builder --spec
fission fn create --name hello --env go --src hello.go --entrypoint Handler --spec
fission fn create --name hello-newdeploy --env go --src hello.go --entrypoint Handler --executortype newdeploy --spec

fission spec apply

• Apply the spec and test it.

fission fn test --name hello
fission fn test --name hello-newdeploy

• Upgrade fission installation to the current code base.

make update-crds
make skaffold-prebuild
skaffold run -p kind
make build-fission-cli
sudo make install-fission-cli

• Test the functions from previous installation and destroy the spec

fission fn test --name hello
fission fn test --name hello-newdeploy
fission spec destroy

• Create a new spec with newly installed fission CLI and test it and destroy it.

fission spec init
fission env create --name go --image ghcr.io/fission/go-env --builder ghcr.io/fission/go-builder --spec
fission fn create --name hello --env go --src hello.go --entrypoint Handler --spec
fission fn create --name hello-newdeploy --env go --src hello.go --entrypoint Handler --executortype newdeploy --spec

fission spec apply

fission fn test --name hello
fission fn test --name hello-newdeploy

fission spec destroy

• Modify the runtime container name in yaml (specs/env-go.yaml) only and keep podSpec container name as it is. Apply the spec and verify the following error.

Error: abort applying resources: error validating specs: 1 errors occurred:
	* runtime container <container-name> does not exist in the pod spec

• Test the env creation with kubectl utility and only builder deployment will be created and you will see an error runtime container <container-name> not found in pod spec in executor logs.

kubectl apply -f specs/env-go.yaml

• Now, modify the podSpec container name as well and apply the spec and test the functions.

fission spec apply

fission fn test --name hello
fission fn test --name hello-newdeploy

Please update the PR with latest code.
Let us know if you encounter any issue.