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 1766287: - "hello-openshift" template application wrong binary format #7186
Bug 1766287: - "hello-openshift" template application wrong binary format #7186
Conversation
@cyril-ui-developer: This pull request references Bugzilla bug 1766287, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@cyril-ui-developer: This pull request references Bugzilla bug 1766287, which is valid. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
831b09d
to
1d87f84
Compare
@@ -229,6 +233,8 @@ apiVersion: apps/v1 | |||
kind: Deployment | |||
metadata: | |||
name: example | |||
annotations: | |||
image.openshift.io/triggers: ${sampleContainerImage} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're using image.openshift.io/triggers
, we should refer to the image stream. This should look like
image.openshift.io/triggers: ${sampleContainerImage} | |
image.openshift.io/triggers: |- | |
[ | |
{ | |
"from": { | |
"kind": "ImageStreamTag", | |
"name": "openshift/hello-openshift:latest" | |
}, | |
"fieldPath": "spec.template.spec.containers[0].image" | |
} | |
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spadgett Updated, thanks.
1d87f84
to
44750c0
Compare
@@ -403,14 +431,26 @@ apiVersion: v1 | |||
kind: Pod | |||
metadata: | |||
name: example | |||
annotations: | |||
image.openshift.io/triggers: |- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work for an individual pod since image
is immutable after the pod is created.
@@ -338,6 +354,17 @@ apiVersion: apps.openshift.io/v1 | |||
kind: DeploymentConfig | |||
metadata: | |||
name: example | |||
annotations: | |||
image.openshift.io/triggers: |- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DeploymentConfigs have first class API support for triggers and don't need to use the annotation.
$ oc explain deploymentconfig.spec.triggers
KIND: DeploymentConfig
VERSION: apps.openshift.io/v1
RESOURCE: triggers <[]Object>
DESCRIPTION:
Triggers determine how updates to a DeploymentConfig result in new
deployments. If no triggers are defined, a new deployment can only occur as
a result of an explicit client update to the DeploymentConfig with a new
LatestVersion. If null, defaults to having a config change trigger.
DeploymentTriggerPolicy describes a policy for a single trigger that
results in a new deployment.
FIELDS:
imageChangeParams <Object>
ImageChangeParams represents the parameters for the ImageChange trigger.
type <string>
Type of the trigger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spadgett Removed the trigger for Pod and DeploymentConfigs.
Do we need to revert the image to openshift/hello-openshift
for Pod and DeploymentConfigs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyril-ui-developer You shouldn't remove the trigger for DeploymentConfigs. Instead update it to use the spec.triggers
instead of the annotation.
I'd just use registry.redhat.io/openshift4/ose-hello-openshift-rhel8
directly for pods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spadgett Updated, thanks! However the status is still ImagePullBackOff
instead of Running
.
c30f13b
to
c7ab555
Compare
c7ab555
to
eee9a0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cyril-ui-developer, spadgett The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@cyril-ui-developer: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@cyril-ui-developer: All pull requests linked via external trackers have merged: Bugzilla bug 1766287 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign @spadgett
Changed the image to
registry.redhat.io/openshift4/ose-hello-openshift-rhel8
but I'm gettingImagePullBackOff
error for pods