Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG get image logic #1379

Open
mateusoliveira43 opened this issue Apr 3, 2024 · 10 comments
Open

BUG get image logic #1379

mateusoliveira43 opened this issue Apr 3, 2024 · 10 comments
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@mateusoliveira43
Copy link
Contributor

Description

OADP code is changed when sent to production, for example

- name: RELATED_IMAGE_VELERO
value: quay.io/konveyor/velero:latest
- name: RELATED_IMAGE_VELERO_RESTORE_HELPER
value: quay.io/konveyor/velero-restore-helper:latest
- name: RELATED_IMAGE_OPENSHIFT_VELERO_PLUGIN
value: quay.io/konveyor/openshift-velero-plugin:latest
- name: RELATED_IMAGE_VELERO_PLUGIN_FOR_AWS
value: quay.io/konveyor/velero-plugin-for-aws:latest
- name: RELATED_IMAGE_VELERO_PLUGIN_FOR_MICROSOFT_AZURE
value: quay.io/konveyor/velero-plugin-for-microsoft-azure:latest
- name: RELATED_IMAGE_VELERO_PLUGIN_FOR_GCP
value: quay.io/konveyor/velero-plugin-for-gcp:latest
- name: RELATED_IMAGE_VELERO_PLUGIN_FOR_CSI
value: quay.io/konveyor/velero-plugin-for-csi:latest
- name: RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN
value: quay.io/konveyor/kubevirt-velero-plugin:v0.2.0
- name: RELATED_IMAGE_MUSTGATHER
value: registry.redhat.io/oadp/oadp-mustgather-rhel8:v1.2

values are changed to production images.

But, these values are not changed

const (
VeleroImage = "quay.io/konveyor/velero:latest"
OpenshiftPluginImage = "quay.io/konveyor/openshift-velero-plugin:latest"
AWSPluginImage = "quay.io/konveyor/velero-plugin-for-aws:latest"
AzurePluginImage = "quay.io/konveyor/velero-plugin-for-microsoft-azure:latest"
GCPPluginImage = "quay.io/konveyor/velero-plugin-for-gcp:latest"
CSIPluginImage = "quay.io/konveyor/velero-plugin-for-csi:latest"
RegistryImage = "quay.io/konveyor/registry:latest"
KubeVirtPluginImage = "quay.io/konveyor/kubevirt-velero-plugin:v0.2.0"
)

Which means, in a production environment, all fallback from below functions, are wrong.

func getVeleroImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.VeleroImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.VeleroImageKey]
}
if os.Getenv("RELATED_IMAGE_VELERO") == "" {
return common.VeleroImage
}
return os.Getenv("RELATED_IMAGE_VELERO")
}

func getAWSPluginImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.AWSPluginImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.AWSPluginImageKey]
}
if os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_AWS") == "" {
return common.AWSPluginImage
}
return os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_AWS")
}
func getCSIPluginImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.CSIPluginImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.CSIPluginImageKey]
}
if os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_CSI") == "" {
return common.CSIPluginImage
}
return os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_CSI")
}
func getGCPPluginImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.GCPPluginImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.GCPPluginImageKey]
}
if os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_GCP") == "" {
return common.GCPPluginImage
}
return os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_GCP")
}
func getOpenshiftPluginImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.OpenShiftPluginImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.OpenShiftPluginImageKey]
}
if os.Getenv("RELATED_IMAGE_OPENSHIFT_VELERO_PLUGIN") == "" {
return common.OpenshiftPluginImage
}
return os.Getenv("RELATED_IMAGE_OPENSHIFT_VELERO_PLUGIN")
}
func getAzurePluginImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.AzurePluginImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.AzurePluginImageKey]
}
if os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_MICROSOFT_AZURE") == "" {
return common.AzurePluginImage
}
return os.Getenv("RELATED_IMAGE_VELERO_PLUGIN_FOR_MICROSOFT_AZURE")
}
func getKubeVirtPluginImage(dpa *oadpv1alpha1.DataProtectionApplication) string {
if dpa.Spec.UnsupportedOverrides[oadpv1alpha1.KubeVirtPluginImageKey] != "" {
return dpa.Spec.UnsupportedOverrides[oadpv1alpha1.KubeVirtPluginImageKey]
}
if os.Getenv("RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN") == "" {
return common.KubeVirtPluginImage
}
return os.Getenv("RELATED_IMAGE_KUBEVIRT_VELERO_PLUGIN")
}

How to reproduce

Install OADP operator from marketplace and change the value of one these environment variables to an empty string (or remove the environment variable). Create DPA that will use one of these images, without using unsupportedOverrides. OADP will try to use a non production image instead of a production one.

Related work for DPA when API version updates

Instead of having unsupportedOverrides field in DPA, could not we use these environment variables to change images used? Instead of a field in DPA, would need to change value in operator CSV.

@kaovilai
Copy link
Member

kaovilai commented Apr 3, 2024

change the value of one these environment variables to an empty string

well that's not really a supported configuration. but I could see how it can be interpreted as a bug.

@mateusoliveira43
Copy link
Contributor Author

@kaovilai is it dangerous somehow to edit that?

@kaovilai
Copy link
Member

kaovilai commented Apr 3, 2024

Not dangerous but not sure why one would make a change there and set it to empty.

@mateusoliveira43
Copy link
Contributor Author

The empty was just to show that there is no need for the fallback value

What I would really like is to remove unsupportedOverrides (in new version of DPA) and change these environment variables instead. It should be as easy as it is to set a value in DPA.

@kaovilai
Copy link
Member

kaovilai commented Apr 3, 2024

sounds good.

@kaovilai
Copy link
Member

kaovilai commented Apr 3, 2024

unsupporetedOverrides would remain for non images

@mateusoliveira43
Copy link
Contributor Author

Yeah, need to see if there is a easy way to change this as well

const OperatorTypeKey UnsupportedImageKey = "operator-type"

@kaovilai
Copy link
Member

kaovilai commented Apr 3, 2024

Let's not change that, it's for MTC.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 3, 2024
@mateusoliveira43
Copy link
Contributor Author

/lifecycle frozen

@openshift-ci openshift-ci bot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

3 participants