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

external: pool and metadata ec pools were reversed in scripts #11919

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

dragon2611
Copy link

Pool and Metadata Pool Appear to be reversed causing PVC creation to fail

Description of your changes:
It seems the data-pool and EC pool on an external cluster are set the wrong way round in the example code for configuring external clusters

dataPool: needs to contain the EC Data pool
pool: needs to contain the Metadata Pool

Which issue is resolved by this Pull Request:
Resolves
#11918

@dragon2611 dragon2611 changed the title External Cluster EC Metadata/Data Pool is reversed causing PVC creation to fail External: EC Metadata/Data Pool is reversed causing PVC creation to fail Mar 16, 2023
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

The changes look good, thanks for the contribution. It appears nobody used the EC pools before for the external clusters.

To make the commitlint bot happy, try the following as a commit title:

external: pool and metadata ec pools were reversed in scripts

@dragon2611 dragon2611 changed the title External: EC Metadata/Data Pool is reversed causing PVC creation to fail external: pool and metadata ec pools were reversed in scripts Mar 16, 2023
@dragon2611
Copy link
Author

Not sure that should have really been a force push on my repo, but I did a git commit --amend to change the commit message, hopefully the bot is happier now.

@travisn
Copy link
Member

travisn commented Mar 16, 2023

Not sure that should have really been a force push on my repo, but I did a git commit --amend to change the commit message, hopefully the bot is happier now.

Looks good now, thanks. Actually the force push is recommended, see our dev guide for future reference.

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Just waiting for the CI now to merge...

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Oh, I overlooked that the DCO check is failing. See the dev guide on the expected signature in the commit message, thanks.

@dragon2611
Copy link
Author

Hopefully it's happier now.

Pool and Metadata Pool Appear to be reversed.

dataPool needs to contain the EC Data pool
pool needs to contain the Metadata Pool
Closes: rook#11918

Signed-off-by: Matt P <dragon@fbdragon.co.uk>
@dragon2611
Copy link
Author

dragon2611 commented Mar 16, 2023

@travisn seems the DCO check is still not happy?
Only thing I can think of is that email doesn't match the github account that it thinks made the commit (which it seems to have taken from one of my SSH public keys)

@travisn
Copy link
Member

travisn commented Mar 16, 2023

@travisn seems the DCO check is still not happy? Only thing I can think of is that email doesn't match the github account that it thinks made the commit (which it seems to have taken from one of my SSH public keys)

It looks like the right format, i'm not sure it validates the email anyway. On occasion it's not clear what it's being picky about, so I set it to ignore for this commit.

@parth-gr
Copy link
Member

So @travisn @dragon2611 we also need similar change here?

@dragon2611
Copy link
Author

@parth-gr That looks ok to me, it seems when using a standard replicated pool you just need pool but for EC you need a standard replicated pool for the metadata and then the EC pool for the data.

dataPool: Should be the EC pool
pool: Should be the replicated metadata pool

@parth-gr
Copy link
Member

agree thanks

travisn added a commit that referenced this pull request Mar 20, 2023
external: pool and metadata ec pools were reversed in scripts (backport #11919)
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.

None yet

4 participants