Skip to content

templates: drop files related to registry #225

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 2 commits into from
Dec 10, 2018
Merged

templates: drop files related to registry #225

openshift-merge-robot merged 2 commits into from
Dec 10, 2018

Conversation

@abhinavdahiya
Copy link
Contributor

@abhinavdahiya abhinavdahiya commented Dec 8, 2018

cluster-image-registry-operator now owns setting these up for access to internal registry.
https://github.com/openshift/cluster-image-registry-operator/blob/master/deploy/08-ca-daemonset.yaml#L26-L46

This fixes MCD and the cluster-image-registry-operator stepping on each other. And MCD currently reboot on syncing any
change which causes disruption on masters recurringly.

Should help dropping ownership of /etc/hosts and /etc/docker.d/certs/... from daemon
#224 (comment)

@abhinavdahiya
templates: drop files related to registry
0efa064 
`cluster-image-registry-operator` now owns setting these up for access to internal registry.
https://github.com/openshift/cluster-image-registry-operator/blob/master/deploy/08-ca-daemonset.yaml#L26-L46

This fixes MCD and the `cluster-image-registry-operator` stepping on each other. And MCD currently reboot on syncing any
change which causes disruption on masters recurringly.
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 8, 2018
@wking wking mentioned this pull request Dec 8, 2018
Closed
@wking
Copy link
Member

wking commented Dec 8, 2018

/lgtm

Offloading registry certs to the registry operator and /etc/hosts to the DNS operator makes sense to me. And if this greens CI, I'm fine landing it without waiting for everyone else to wake up ;). We can always revert in the morning if it doesn't hold up to sunlight 🦇 😉

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2018
@wking
Copy link
Member

wking commented Dec 8, 2018

Hrm, router didn't come up:

timeout waiting for openshift-ingress/deploy/router-default to be available
2018/12/08 09:37:40 Container test in pod e2e-aws failed, exit code 1, reason Error

Maybe there's a chicken/egg with one of these files?

@wking
Copy link
Member

wking commented Dec 8, 2018

/retest

Just in case we get lucky...

@crawford
Copy link
Contributor

crawford commented Dec 9, 2018

/retest

@wking
Copy link
Member

wking commented Dec 9, 2018

So one potential issue with being able to unilaterally merge this would be that, as described in openshift/cluster-dns-operator#63, the DNS operator is currently just appending to /etc/hosts. It will need to start managing the file. Does the content we're removing here differ from what RHCOS ships?

@wking wking mentioned this pull request Dec 9, 2018
Closed
@abhinavdahiya
Copy link
Contributor Author

abhinavdahiya commented Dec 9, 2018

/retest

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2018
@wking
Copy link
Member

wking commented Dec 9, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2018
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2018
@abhinavdahiya
Copy link
Contributor Author

abhinavdahiya commented Dec 9, 2018

openshift/cluster-storage-operator#5

Need this ^^

@wking
Copy link
Member

wking commented Dec 10, 2018

/override ci/prow/e2e-aws

@crawford crawford closed this Dec 10, 2018
@crawford crawford reopened this Dec 10, 2018
@crawford
Copy link
Contributor

crawford commented Dec 10, 2018

Fat-fingered it.

Should we remove the debug commit?

@crawford
Copy link
Contributor

crawford commented Dec 10, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2018
@crawford
Copy link
Contributor

crawford commented Dec 10, 2018

/lgtm
/override ci/prow/e2e-aws

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 10, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, crawford, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,crawford,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wking
Copy link
Member

wking commented Dec 10, 2018

e2e-aws:

level=fatal msg="Error executing openshift-install: waiting for bootstrap-complete: timed out waiting for the condition"

@wking
Copy link
Member

wking commented Dec 10, 2018

/retest

@smarterclayton
Copy link
Contributor

smarterclayton commented Dec 10, 2018

Why is that oc error being printed?

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2018
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 10, 2018

New changes are detected. LGTM label has been removed.

@abhinavdahiya
Copy link
Contributor Author

abhinavdahiya commented Dec 10, 2018 •
edited
Loading

force pushed as previously i have seen the release image being reused in same namespace. so that i can get the changes from openshift/cluster-storage-operator#5

@abhinavdahiya abhinavdahiya added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2018
@smarterclayton
Copy link
Contributor

smarterclayton commented Dec 10, 2018

Link?

@smarterclayton
Copy link
Contributor

smarterclayton commented Dec 10, 2018

Ah, you aren’t getting a new release image when other tags change (outside of this project). Might be time to change that.

@abhinavdahiya
Copy link
Contributor Author

abhinavdahiya commented Dec 10, 2018 •
edited
Loading

Ah, you aren’t getting a new release image when other tags change (outside of this project). Might be time to change that.

Yeah, in one of my PRs in this repo i vaguely remember something like that happening.. i don't remember which 😇

@openshift-merge-robot openshift-merge-robot merged commit 1b2309b into openshift:master Dec 10, 2018
@abhinavdahiya abhinavdahiya deleted the drop_registry_files branch December 10, 2018 05:25
@abhinavdahiya abhinavdahiya mentioned this pull request Dec 10, 2018
Merged
This was referenced Dec 14, 2018
Closed
Closed
osherdp pushed a commit to osherdp/machine-config-operator that referenced this pull request Apr 13, 2021
@gabemontero
Merge pull request openshift#225 from gabemontero/more-s390
0038cc3 
enable sample imagestreams and templates on Z
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crawford crawford Awaiting requested review from crawford

@jlebon jlebon Awaiting requested review from jlebon

Assignees

@wking wking

@crawford crawford

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@abhinavdahiya @wking @crawford @openshift-ci-robot @smarterclayton @openshift-merge-robot