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

🌲 FIX - broken selector on service and updated nginx image 🌳 #148

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

springdo
Copy link
Contributor

@springdo springdo commented Jul 7, 2020

What does this PR do?

Two fixes. Happy to raise a PR for each though ;)

  • nginx 112 is deprecated and no longer exists in OpenShift 4.4 so this pipeline will fail. I bumped it to 114
  • Since merging Adopt standardized app labels for the basic-nginx app #145 the app=${APPLICATION_NAME} selector was removed so the service connection to the pod is broken. This fixes that by only using the deploymentConfig.

FYI - @etsauer I mentioned this in the Pelorus Dev channel.

How should this be tested?

See README.

Who would you like to review this?

cc: @redhat-cop/containers-approvers

@springdo
Copy link
Contributor Author

springdo commented Jul 8, 2020

/assign @etsauer

@etsauer
Copy link
Contributor

etsauer commented Jul 8, 2020

@springdo the problem with this is that nginx:1.14 doesn't seem to exist in 4.3, so this breaks in that case. I find it odd that from one version to the next, there is no overlap in the tags.

$ oc get clusterversion
NAME      VERSION   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.3.18    True        False         56d     Cluster version is 4.3.18

$ oc get imagestream -n openshift nginx
NAME    IMAGE REPOSITORY                                                                   TAGS               UPDATED
nginx   default-route-openshift-image-registry.apps.d1.casl.rht-labs.com/openshift/nginx   1.10,1.12,latest   4 months ago

@springdo
Copy link
Contributor Author

springdo commented Jul 9, 2020

@etsauer yup that's annoying alright. I noticed that the image is not in the UI for the registry but you can still pull it.

$ oc get clusterversion
NAME      VERSION   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.4.3     True        False         55d     Cluster version is 4.4.3

$ oc get imagestream -n openshift nginx
NAME    IMAGE REPOSITORY                                                   TAGS               UPDATED
nginx   image-registry.openshift-image-registry.svc:5000/openshift/nginx   1.10,1.14,latest   8 weeks ago

$ docker pull registry.access.redhat.com/rhscl/nginx-112-rhel7
Using default tag: latest
latest: Pulling from rhscl/nginx-112-rhel7
Digest: sha256:3701ac560e3f005b219a7bdfe51c59909c4899ca1a3409f3ac57e7dde9462cdd
Status: Image is up to date for registry.access.redhat.com/rhscl/nginx-112-rhel7:latest
registry.access.redhat.com/rhscl/nginx-112-rhel7:latest

Do you want me to split the PR up so? One for the selector fix and one for the nginx fix so we can at least unbreak the deployment?

@etsauer
Copy link
Contributor

etsauer commented Jul 9, 2020

@springdo what if, for now, we include our own imagestream that contains 1.14, so that its available no matter what?

@stale
Copy link

stale bot commented Sep 7, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Sep 7, 2020
@stale stale bot closed this Sep 14, 2020
@etsauer etsauer reopened this Oct 13, 2020
@stale stale bot removed the wontfix label Oct 13, 2020
@springdo
Copy link
Contributor Author

@etsauer - fyi

Copy link
Contributor

@etsauer etsauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm for now. we'll come back as a followup and make it work for older versions of OCP

@etsauer
Copy link
Contributor

etsauer commented Oct 13, 2020

/lgtm /approve

@etsauer
Copy link
Contributor

etsauer commented Oct 13, 2020

/approve

@etsauer
Copy link
Contributor

etsauer commented Oct 13, 2020

/lgtm

@redhat-cop-ci-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: etsauer

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@redhat-cop-ci-bot redhat-cop-ci-bot merged commit b9c8a1f into master Oct 13, 2020
@redhat-cop-ci-bot redhat-cop-ci-bot deleted the fix/nginx branch October 13, 2020 14:04
joecharles33 added a commit to joecharles33/container-pipelines that referenced this pull request Oct 13, 2020
🌲 FIX - broken selector on service and updated nginx image 🌳 (redhat-cop#148)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants