[GitHub] [incubator-yunikorn-k8shim] craigcondit commented on a change in pull request #381: [YUNIKORN-638] Make placeholder image configurable

[GitBox <gi...@apache.org>]

craigcondit commented on a change in pull request #381:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/381#discussion_r823002052



##########
File path: pkg/cache/placeholder.go
##########
@@ -104,3 +108,20 @@ func (p *Placeholder) String() string {
 	return fmt.Sprintf("appID: %s, taskGroup: %s, podName: %s/%s",
 		p.appID, p.taskGroupName, p.pod.Namespace, p.pod.Name)
 }
+
+func getPlaceHolderImage() string {
+	if placeHolderImage == "" {
+		imageEnvSetting := os.Getenv(constants.PlaceHolderImageEnvVar)

Review comment:
       Let's use the standard configuration mechanism we already have in place rather than ENV vars here (those should be handled in the Docker container config and passed as arguments). Be sure to update both the shim and plugin setups.

##########
File path: pkg/cache/placeholder.go
##########
@@ -104,3 +108,20 @@ func (p *Placeholder) String() string {
 	return fmt.Sprintf("appID: %s, taskGroup: %s, podName: %s/%s",
 		p.appID, p.taskGroupName, p.pod.Namespace, p.pod.Name)
 }
+
+func getPlaceHolderImage() string {
+	if placeHolderImage == "" {
+		imageEnvSetting := os.Getenv(constants.PlaceHolderImageEnvVar)
+		if imageEnvSetting != "" {
+			placeHolderImage = imageEnvSetting
+			log.Logger().Info("Using placeholder image from environment variable", zap.String("imageName",

Review comment:
       I think we don't need to log here but can just use the config printout that comes in from the shim startup already.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org