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.

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

Render BMO deployment #672

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Render BMO deployment #672

merged 1 commit into from
Oct 8, 2020

Conversation

kashifest
Copy link
Member

This PR adds a render of BMO deployment.

It takes care of the task done at #667

@metal3-io-bot metal3-io-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 8, 2020
@kashifest
Copy link
Member Author

/test-integration
/assign @maelk
/cc @digambar15

maelk
maelk suggested changes Oct 8, 2020
View reviewed changes
Copy link
Member

@maelk maelk left a comment

Choose a reason for hiding this comment

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

Please add the rendered file in the hack/generate.sh file so that it is checked by the job

config/render/deployment.yaml Show resolved Hide resolved
@kashifest
Copy link
Member Author

kashifest commented Oct 8, 2020 •
edited

Please add the rendered file in the hack/generate.sh file so that it is checked by the job

It is already checked in generate.sh the whole config dir.

@maelk
Copy link
Member

maelk commented Oct 8, 2020

It is already checked in generate.sh the whole config dir.

oh true! sorry

digambar15
digambar15 reviewed Oct 8, 2020
View reviewed changes
Copy link

@digambar15 digambar15 left a comment

Choose a reason for hiding this comment

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

Otherwise looks good

config/render/deployment.yaml
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
Copy link

Choose a reason for hiding this comment

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

Didn't you face metadata error because it raises error when you apply kustomize on rendered file ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't get you, do you suggest a change here?

Copy link

Choose a reason for hiding this comment

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

No problem. That's fine. I can see change in manager.yaml.

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2020
@kashifest
Copy link
Member Author

/cc @dhellmann @stbenjam

@kashifest
Copy link
Member Author

/test-integration

digambar15
digambar15 approved these changes Oct 8, 2020
View reviewed changes
config/render/deployment.yaml
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
Copy link

Choose a reason for hiding this comment

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

No problem. That's fine. I can see change in manager.yaml.

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: digambar15, kashifest, maelk

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

@maelk
Copy link
Member

maelk commented Oct 8, 2020

Getting this in to unlock work on CAPM3 side. The namespace change is required because the recent version of Kustomize does not apply the "namespace" kustomization to the namespace object name itself.

/lgtm

If there are any comments, we will address them in a second PR.

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2020
@metal3-io-bot metal3-io-bot merged commit e4fcc9b into metal3-io:master Oct 8, 2020
@dhellmann
Copy link
Member

This looks fine, but for my own education can you confirm that we're checking this rendered file in because CI jobs use it?

@kashifest
Copy link
Member Author

Yes, this rendered file is here to make sure that duplication of name of the objects doesn't collide while we kustomize all resources in CI

@dhellmann
Copy link
Member

Yes, this rendered file is here to make sure that duplication of name of the objects doesn't collide while we kustomize all resources in CI

Great, thank you for explaining that!

@maelk
Copy link
Member

maelk commented Oct 9, 2020

It's actually not limited to CI. Whenever we run the kustomization in CAPM3, we are now using this file.

@maelk maelk mentioned this pull request Dec 14, 2020
Closed
@kashifest kashifest deleted the create/bmo-deployment-yaml branch October 5, 2023 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digambar15 digambar15 digambar15 approved these changes

@maelk maelk maelk approved these changes

@dhellmann dhellmann Awaiting requested review from dhellmann

@stbenjam stbenjam Awaiting requested review from stbenjam

Assignees

@maelk maelk

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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kashifest @maelk @metal3-io-bot @dhellmann @digambar15