Bug 1957227

Summary: Issue replacing the EnvVariables using the unsupported ConfigMap
Product: OpenShift Container Platform Reporter: Juan Manuel Parrilla Madrid <jparrill>
Component: assisted-installerAssignee: Mat Kowalski <mko>
assisted-installer sub component: Deployment Operator QA Contact: bjacot
Status: CLOSED ERRATA Docs Contact:
Severity: medium    
Priority: medium CC: aos-bugs, fpercoco, trwest
Version: 4.8Keywords: Triaged
Target Milestone: ---   
Target Release: 4.8.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: OCP-Metal-v1.0.21.1 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2021-07-27 23:06:36 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Juan Manuel Parrilla Madrid 2021-05-05 12:15:51 UTC 
Description of problem:
Using the capability from https://github.com/openshift/assisted-service/blob/master/docs/operator.md#deploying-the-operand to deploy an AssistedServiceConfig with the annotation "unsupported.agent-install.openshift.io/assisted-service-configmap", the assisted-service deployment has the right envFrom ConfigMap but the variables that already exists and are defined, are not overrided, the variables that doesn't exists are newly created on the pod.

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. Create the configMap containing the AI envVars
apiVersion: v1
kind: ConfigMap
metadata:
  name: assisted-service-config
  namespace: "assisted-installer"
  labels:
    app: assisted-service
data:
  OPENSHIFT_VERSIONS: '{"4.8":{"rhcos_image":"http://[2620:52:0:1303::1]/rhcos-48.84.202104252100-0-live.x86_64.iso","rhcos_version":"48.84.202104252100-0"}}'
  SERVE_HTTPS: 'False'
  AUTH_TYPE: 'none'


2. Create the Operand
---
apiVersion: agent-install.openshift.io/v1beta1
kind: AgentServiceConfig
metadata:
  namespace: assisted-installer
  name: agent
  annotations:
    unsupported.agent-install.openshift.io/assisted-service-configmap: 'assisted-service-config'
spec:
  databaseStorage:
    accessModes:
    - ReadWriteOnce
    resources:
      requests:
        storage: 40Gi
  filesystemStorage:
    accessModes:
    - ReadWriteOnce
    resources:
      requests:
        storage: 40Gi
  mirrorRegistryRef:
    name: 'hyper1-mirror-config'

3. execute an oc rsh into the pod and grep by the HTTPS

oc rsh deployment/assisted-service
env | grep SERVE_HTTPS


Actual results:
SERVE_HTTPS=True

Expected results:
SERVE_HTTPS=False

Additional info:
assisted-service operator tag: 262e28709c491f58c2ddadd0dec24c9ce6b5e93d
Comment 1 Antoni Segura Puimedon 2021-05-05 12:21:40 UTC 
The problem is that the operator sets the default values for the env variables, and those, according to the Kubernetes API reference:

"Values defined by an Env with a duplicate key will take precedence"

"When a key exists in multiple sources, the value associated with the last source will take precedence."

Thus, we should make sure that we either do not provide the default value for those configs in the ConfigMap OR we should just override the values in Env like suggested in https://stackoverflow.com/a/54398918
Comment 2 Flavio Percoco 2021-05-05 12:41:54 UTC 
> Thus, we should make sure that we either do not provide the default value for those configs in the ConfigMap OR we should just override the values in Env like suggested in 

I don't think we are at the point where we can skip setting some of these default values so I would be inclined to say that we want to override them. It also aligns with what this annotation is communicating:

1. Use of this annotation is not supported
2. Use it at your own risk
3. Behavior, when using this annotation, is considered unexpected and untested.

Overriding the env makes sense as that's the whole point of the annotation; to be able to set custom envs as needed and override the default/recommended behavior.
Comment 3 Mat Kowalski 2021-05-07 07:54:31 UTC 
We have discussed internally and the final decision on how to proceed is as follows

1. Put all of the existing environment variables into a configmap owned and managed by the asc_controller that gets mounted as envFrom into the assisted-service container.
2. Make sure that the configmap annotation envFrom always comes after the configmap "we" own.
Comment 6 Trey West 2021-06-25 15:41:23 UTC 
Verified 

# oc get cm/assisted-service-config -o json | jq -r '.data'
{
  "AUTH_TYPE": "none"
}

# oc rsh deployment/assisted-service
Defaulted container "assisted-service" out of: assisted-service, postgres
sh-4.4$ env | grep AUTH_TYPE
AUTH_TYPE=none
Comment 8 errata-xmlrpc 2021-07-27 23:06:36 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: OpenShift Container Platform 4.8.2 bug fix and security update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2021:2438