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

Dedupe on-prem templates #2079

Merged
merged 4 commits into from Nov 2, 2020

Conversation

bcrochet
Copy link
Member

There are a number of duplicated files across the on-prem platforms.
This patch begins the process of de-duplicating them. This is going
to be a bigger task than the bootstrap manifests, as not all of the
files are duplicated, and some have differences that would not lend
to being deduped. But this at least starts small, and proves the
concept.

- What I did

- How to verify it

- Description for the changelog

Dedupe templates for on-prem platforms

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 13, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 14, 2020
@pierreprinetti
Copy link
Member

/test e2e-openstack

@openshift-ci-robot
Copy link
Contributor

@bcrochet: The label(s) /label 4.7 cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved

In response to this:

/label 4.7

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.

@bcrochet
Copy link
Member Author

/label platform/baremetal
/label platform/openstack

@openshift-ci-robot
Copy link
Contributor

@bcrochet: The label(s) platform/baremetal, platform/openstack cannot be applied, because the repository doesn't have them

In response to this:

/label platform/baremetal
/label platform/openstack

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.

@cgwalters cgwalters added the 4.7 Work deferred for 4.7 label Sep 17, 2020
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 25, 2020
@cgwalters cgwalters mentioned this pull request Sep 25, 2020
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Only skimmed this I think the lines deleted count looks awesome!

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2020
@bcrochet bcrochet force-pushed the templates-dedupe branch 2 times, most recently from a78a161 to fedc2c7 Compare October 12, 2020 14:23
@bcrochet
Copy link
Member Author

/test e2e-ovirt
/test e2e-openstack
/test e2e-vsphere

@openshift-ci-robot
Copy link
Contributor

@bcrochet: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-gcp-upgrade f9f8770b2b099345162b7a1e6dd00d4c17fd4ce7 link /test e2e-gcp-upgrade
ci/prow/e2e-aws fedc2c7a617786709ef93e82ba3ce71bd95afa29 link /test e2e-aws
ci/prow/e2e-ovn-step-registry fedc2c7a617786709ef93e82ba3ce71bd95afa29 link /test e2e-ovn-step-registry
ci/prow/e2e-aws-workers-rhel7 fedc2c7a617786709ef93e82ba3ce71bd95afa29 link /test e2e-aws-workers-rhel7
ci/prow/e2e-ovirt fedc2c7a617786709ef93e82ba3ce71bd95afa29 link /test e2e-ovirt

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.

@bcrochet
Copy link
Member Author

/retest

@bcrochet
Copy link
Member Author

/test e2e-ovirt
/test e2e-openstack
/test e2e-vsphere

@bcrochet
Copy link
Member Author

/retest

@ravidbro
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@ravidbro
Copy link

ravidbro commented Nov 1, 2020

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 1, 2020
@ravidbro
Copy link

ravidbro commented Nov 1, 2020

I put /hold to stop the bot from testing every hour until the conflict will be resolved

@ravidbro
Copy link

ravidbro commented Nov 2, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

There are a number of duplicated files across the on-prem platforms.
This patch begins the process of de-duplicating them. This is going
to be a bigger task than the bootstrap manifests, as not all of the
files are duplicated, and some have differences that would not lend
to being deduped. But this at least starts small, and proves the
concept.
Next step in the deduplication of templates. This targets the NetworkManager
files.
This does a dedupe of the CoreDNS related templates. This also has the effect
of proving out the "replace with more specific template" option.
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2020
@ravidbro
Copy link

ravidbro commented Nov 2, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2020
Signed-off-by: Ravid Brown <ravid@redhat.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2020
@chenyosef
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bcrochet, cgwalters, chenyosef, ravidbro

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

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@ravidbro
Copy link

ravidbro commented Nov 2, 2020

/retest

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Nov 2, 2020

@bcrochet: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-vsphere 51ff38290a2b2af8f2114c08621c980b22844b9f link /test e2e-vsphere
ci/prow/e2e-ovirt 51ff38290a2b2af8f2114c08621c980b22844b9f link /test e2e-ovirt
ci/prow/e2e-ovn-step-registry 1236612 link /test e2e-ovn-step-registry

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.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 502f5bd into openshift:master Nov 2, 2020
patrickdillon added a commit to patrickdillon/installer that referenced this pull request Nov 5, 2020
This creates an empty infrastructure.Status.PlatformStatus.VSphere
object during UPI installs. Prior to this change, the object has been
nil during UPI installs. The object is optional, but a recent change
to MCO templating[0] has a dependency on the object.

[0]: openshift/machine-config-operator#2079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.7 Work deferred for 4.7 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants