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

Node labelling UX #726

Merged
merged 7 commits into from
Jan 30, 2024
Merged

Node labelling UX #726

merged 7 commits into from
Jan 30, 2024

Conversation

tiagolobocastro
Copy link
Contributor

@tiagolobocastro tiagolobocastro commented Jan 26, 2024

❯ rest-plugin label node io-engine-1 openebs.io/performance=good
Node io-engine-1 labelled successfully. Current labels: {"openebs.io/performance": "good"}

❯ rest-plugin label node io-engine-1 openebs.io/performance=good
Node io-engine-1 not labelled as the label key already exists

❯ rest-plugin label node io-engine-1 openebs.io/performance=bad
Node io-engine-1 not labelled as the label already exists with different value

❯ rest-plugin label node io-engine-1 openebs.io/performance/also=good
Invalid node label: / is only allowed once

❯ rest-plugin label node io-engine-1 openebs.io/performance/=good
Invalid node label: / can only be used in the middle of the key

❯ rest-plugin label node io-engine-1 openebs.io/performance
Invalid node label: At least one label value is required

❯ rest-plugin label node io-engine-1 openebs.io/performance-
Node io-engine-1 labelled successfully. Current labels: {}

❯ rest-plugin label node io-engine-1 openebs.io/performances-
Node io-engine-1 not unlabelled as it did not contain the labe


refactor: apply python linter suggestions

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

ci: check if python linter modified files

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

ci: don't install pre-commit on CI

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

refactor(node-label/api): doc the cli and openapi with key val

Expands the parsing of label on the cli and returns different error messages.
Splits the label into key and value.
todo: is this the right approach here, where should we do the validation?
kubectl seems to some validation, is there anything on the apiserver too?

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

refactor(node-label): use specific types for store operations

PStor operations should use a specific type to allow for better expansion.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

docs(node-label/rest-plugin): improve ux for the command line

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

build(openapi-generator): upgrade to v6.4.0

Namely fixes a bug related to path parsing with '='

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

Namely fixes a bug related to path parsing with '='

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
PStor operations should use a specific type to allow for better expansion.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Copy link
Member

@Abhinandan-Purkait Abhinandan-Purkait left a comment

Choose a reason for hiding this comment

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

LGTM

control-plane/plugin/src/resources/mod.rs Outdated Show resolved Hide resolved
control-plane/plugin/src/resources/mod.rs Show resolved Hide resolved
control-plane/plugin/src/resources/node.rs Outdated Show resolved Hide resolved
Copy link
Member

@sinhaashish sinhaashish left a comment

Choose a reason for hiding this comment

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

LGTM

@tiagolobocastro tiagolobocastro force-pushed the label-openapi branch 2 times, most recently from 289a8ef to 1b2128c Compare January 29, 2024 22:56
@tiagolobocastro
Copy link
Contributor Author

bors merge

bors-openebs-mayastor bot pushed a commit that referenced this pull request Jan 29, 2024
726: Node labelling UX r=tiagolobocastro a=tiagolobocastro

❯ rest-plugin label node io-engine-1 openebs.io/performance=good
Node io-engine-1 labelled successfully. Current labels: {"openebs.io/performance": "good"}

❯ rest-plugin label node io-engine-1 openebs.io/performance=good
Node io-engine-1 not labelled as the label key already exists

❯ rest-plugin label node io-engine-1 openebs.io/performance=bad
Node io-engine-1 not labelled as the label key already exists, but with a different value and --overwrite is false

❯ rest-plugin label node io-engine-1 openebs.io/performance/also=good
Invalid label format: key can contain at most one / character

❯ rest-plugin label node io-engine-1 performance/=good
Invalid label format: both key and value parts must start with an ascii alphanumeric character

❯ rest-plugin label node io-engine-1 openebs.io/performance
Invalid label format: the supported formats are: key=value for adding (example: group=a) and key- for removing (example: group-)

❯ rest-plugin label node io-engine-1 openebs.io/performance-
Node io-engine-1 labelled successfully. Current labels: {}

❯ rest-plugin label node io-engine-1 openebs.io/performances-
Node io-engine-1 not unlabelled as it did not contain the label

---

    refactor: apply python linter suggestions
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    ci: check if python linter modified files
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    ci: don't install pre-commit on CI
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>


---

    refactor(node-label/api): doc the cli and openapi with key val
    
    Expands the parsing of label on the cli and returns different error messages.
    Splits the label into key and value.
    todo: is this the right approach here, where should we do the validation?
    kubectl seems to some validation, is there anything on the apiserver too?
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    refactor(node-label): use specific types for store operations
    
    PStor operations should use a specific type to allow for better expansion.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    docs(node-label/rest-plugin): improve ux for the command line
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    build(openapi-generator): upgrade to v6.4.0
    
    Namely fixes a bug related to path parsing with '='
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>


Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>
@bors-openebs-mayastor
Copy link

Build failed:

Expands the parsing of label on the cli and returns different error messages.
Splits the label into key and value.
todo: is this the right approach here, where should we do the validation?
kubectl seems to some validation, is there anything on the apiserver too?

Included a bunch of formatting changes due to bad formatted code being
checked in...
todo: run python linter on CI

Updates openapi once again to get some ql error helpers.

Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
@tiagolobocastro
Copy link
Contributor Author

bors merge

@bors-openebs-mayastor
Copy link

Build succeeded:

@bors-openebs-mayastor bors-openebs-mayastor bot merged commit 5493f7d into develop Jan 30, 2024
4 checks passed
@bors-openebs-mayastor bors-openebs-mayastor bot deleted the label-openapi branch January 30, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants