Skip to content

doc3721: expose LH resource reservation to harvester-configuration#301

Merged
guangbochen merged 2 commits intoharvester:mainfrom
w13915984028:doc3721
Aug 1, 2023
Merged

doc3721: expose LH resource reservation to harvester-configuration#301
guangbochen merged 2 commits intoharvester:mainfrom
w13915984028:doc3721

Conversation

@w13915984028
Copy link
Member

@w13915984028 w13915984028 commented Apr 12, 2023 •
edited
Loading

expose LH resource reservation to harvester-configuration
expose default storage class replica count to harvester-configuration

issue:
harvester/harvester#3721

source code PR:
Original PR ( harvester/harvester-installer#466 ) is merged into harvester/harvester-installer#402

epic:
harvester/harvester#3262

@netlify
Copy link

netlify bot commented Apr 12, 2023 •
edited
Loading

✅ Deploy Preview for harvester-preview ready!

Name Link
🔨 Latest commit 787fefe
🔍 Latest deploy log https://app.netlify.com/sites/harvester-preview/deploys/64c87a125108060008fadf1c
😎 Deploy Preview https://deploy-preview-301--harvester-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@w13915984028 w13915984028 mentioned this pull request Apr 12, 2023
Closed
@w13915984028 w13915984028 mentioned this pull request Apr 12, 2023
Closed
@w13915984028 w13915984028 changed the title expose LH resource reservation to harvester-configuration doc3721: expose LH resource reservation to harvester-configuration Apr 12, 2023
@w13915984028 w13915984028 mentioned this pull request Apr 12, 2023
Merged
vickyhella
vickyhella reviewed Apr 13, 2023
View reviewed changes
Copy link
Contributor

@vickyhella vickyhella left a comment

Choose a reason for hiding this comment

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

Just some small suggestions

@w13915984028 w13915984028 requested a review from vickyhella April 13, 2023 12:21
@LucasSaintarbor LucasSaintarbor added this to the v1.2.0 milestone Jun 6, 2023
guangbochen
guangbochen approved these changes Jun 30, 2023
View reviewed changes
Copy link
Contributor

@guangbochen guangbochen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

guangbochen
guangbochen reviewed Jun 30, 2023
View reviewed changes
docs/install/harvester-configuration.md Outdated
data_disk: /dev/sdb
```

### `install.harvester.storageClass.replicaCount`
Copy link
Contributor

@guangbochen guangbochen Jun 30, 2023

Choose a reason for hiding this comment

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

please use snake_case instead.

Copy link
Member Author

@w13915984028 w13915984028 Jul 3, 2023

Choose a reason for hiding this comment

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

What is the consideration for us to stick to snake_case , is it mainly for python scripts to work correctly? thanks.

From below post, seems in k8s world, the camelCase is more preferred.

https://stackoverflow.com/questions/22771226/what-is-the-canonical-yaml-naming-style

Copy link
Contributor

@guangbochen guangbochen Jul 5, 2023

Choose a reason for hiding this comment

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

this is an inherited design from k3os(harvester's original baseOS), and making it consistent with the existing doc format.

@w13915984028 w13915984028 mentioned this pull request Jul 3, 2023
Closed
@guangbochen guangbochen force-pushed the doc3721 branch 2 times, most recently from afbce77 to 68ccd00 Compare August 1, 2023 03:01
w13915984028 and others added 2 commits August 1, 2023 11:14
@w13915984028 @guangbochen
expose LH resource reservation to harvester-configuration
72820fd 
expose default storage class replica count to harvester-configuration

Signed-off-by: Jian Wang <w13915984028@gmail.com>
@guangbochen
update new config formart and fix desc
787fefe
@guangbochen
Copy link
Contributor

guangbochen commented Aug 1, 2023

verified new cluster install with the following config, and it works as expected with those new config:

scheme_version: 1
install:
  data_disk: /dev/disk/by-id/scsi-0HP_LOGICAL_VOLUME_01000000
  harvester:
    storage_class:
      replica_count: 1
    longhorn:
      default_settings:
        guaranteedReplicaManagerCPU: 6
        guaranteedEngineManagerCPU: 6
image image

bk201
bk201 approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@guangbochen
Copy link
Contributor

guangbochen commented Aug 1, 2023

@w13915984028 we will merge this PR and feel free to leave a comment if u see any problems, thanks.

@guangbochen guangbochen merged commit 40acb01 into harvester:main Aug 1, 2023
@lanfon72 lanfon72 mentioned this pull request Dec 4, 2023
Closed
@w13915984028 w13915984028 deleted the doc3721 branch July 12, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bk201 bk201 bk201 approved these changes

+2 more reviewers

@guangbochen guangbochen guangbochen approved these changes

@vickyhella vickyhella vickyhella approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@w13915984028 @guangbochen @bk201 @vickyhella @LucasSaintarbor