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

cleanup tests and bash indentation #996

Merged
merged 3 commits into from
Feb 1, 2022
Merged

cleanup tests and bash indentation #996

merged 3 commits into from
Feb 1, 2022

Conversation

harshavardhana
Copy link
Member

@harshavardhana harshavardhana commented Jan 30, 2022 •
edited
Loading

No description provided.

@harshavardhana harshavardhana force-pushed the move-goyaml-instead branch 4 times, most recently from 8fce54c to 4863b72 Compare January 30, 2022 22:02
kanagarajkm
kanagarajkm previously approved these changes Jan 31, 2022
View reviewed changes
dvaldivia
dvaldivia previously requested changes Feb 1, 2022
View reviewed changes
Copy link
Collaborator

@dvaldivia dvaldivia left a comment

Choose a reason for hiding this comment

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

The output of serializing with gopkg.in/yaml.v2 is radically different from the one from sigs.k8s.io/yaml, and the output is incompatible with kubectl apply -f so not good

Screen Shot 2022-01-31 at 6 23 31 PM

@harshavardhana harshavardhana force-pushed the move-goyaml-instead branch 2 times, most recently from 2a7bcf7 to 9420e9d Compare February 1, 2022 02:32
@harshavardhana harshavardhana force-pushed the move-goyaml-instead branch 4 times, most recently from f328bad to df87179 Compare February 1, 2022 02:54
@harshavardhana harshavardhana changed the title replace sigs/yaml to go/yaml for a more spec compliant yaml library @harshavardhana cleanup tests and bash indentation Feb 1, 2022
@harshavardhana harshavardhana changed the title @harshavardhana cleanup tests and bash indentation cleanup tests and bash indentation Feb 1, 2022
@harshavardhana harshavardhana force-pushed the move-goyaml-instead branch 4 times, most recently from 4289649 to d11a3ae Compare February 1, 2022 03:15
dvaldivia
dvaldivia previously approved these changes Feb 1, 2022
View reviewed changes
@harshavardhana
Copy link
Member Author

PTAL @dvaldivia @kanagarajkm

@kanagarajkm
Copy link
Contributor

@harshavardhana prometheus is failing with following error

ts=2022-02-01T14:10:12.900Z caller=main.go:434 level=error msg="Error loading config (--config.file=/etc/prometheus/prometheus.yml)" err="parsing YAML file /etc/prometheus/prometheus.yml: yaml: unmarshal errors:\n  line 4: field Global not found in type config.plain\n  line 7: field ScrapeConfigs not found in type config.plain"

apiVersion: v1
data:
  prometheus.yml: |
    # This file and config-map is generated by MinIO Operator.
    # DO NOT EDIT.

    Global:
      EvaluationInterval: 30000000000
      ScrapeInterval: 30000000000
    ScrapeConfigs:
    - BearerToken: eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJleHAiOjQ3OTczMjQ0MjIsImlzcyI6InByb21ldGhldXMiLCJzdWIiOiIwSUkwS0lMVjJLWldZVFFKIn0.GUY1Syy4S02imTlfldA5Dxnrw45joUCALYXF3s_rFk4SVYC293oPrcCtbH1J-N2KxS9HyvWX0VWGssQ2VcUcpg
      JobName: minio-job
      MetricsPath: /minio/v2/metrics/cluster
      Scheme: https
      StaticConfigs:
      - Targets:
        - minio.tenant1.svc.cluster.local:443
      TLSConfig:
        CAFile: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
kind: ConfigMap
metadata:
  creationTimestamp: "2022-02-01T14:07:02Z"
  name: tenant1-prometheus-config-map
  namespace: tenant1
  ownerReferences:
  - apiVersion: minio.min.io/v2
    blockOwnerDeletion: true
    controller: true
    kind: Tenant
    name: tenant1
    uid: 0a5c90aa-44a7-4ec9-80b1-e2236ae0b35d
  resourceVersion: "160755"
  uid: e2aea3cb-7119-42e0-b4eb-5314220bb8d1

@harshavardhana
Copy link
Member Author

@harshavardhana prometheus is failing with following error

ts=2022-02-01T14:10:12.900Z caller=main.go:434 level=error msg="Error loading config (--config.file=/etc/prometheus/prometheus.yml)" err="parsing YAML file /etc/prometheus/prometheus.yml: yaml: unmarshal errors:\n  line 4: field Global not found in type config.plain\n  line 7: field ScrapeConfigs not found in type config.plain"

apiVersion: v1
data:
  prometheus.yml: |
    # This file and config-map is generated by MinIO Operator.
    # DO NOT EDIT.

    Global:
      EvaluationInterval: 30000000000
      ScrapeInterval: 30000000000
    ScrapeConfigs:
    - BearerToken: eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJleHAiOjQ3OTczMjQ0MjIsImlzcyI6InByb21ldGhldXMiLCJzdWIiOiIwSUkwS0lMVjJLWldZVFFKIn0.GUY1Syy4S02imTlfldA5Dxnrw45joUCALYXF3s_rFk4SVYC293oPrcCtbH1J-N2KxS9HyvWX0VWGssQ2VcUcpg
      JobName: minio-job
      MetricsPath: /minio/v2/metrics/cluster
      Scheme: https
      StaticConfigs:
      - Targets:
        - minio.tenant1.svc.cluster.local:443
      TLSConfig:
        CAFile: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
kind: ConfigMap
metadata:
  creationTimestamp: "2022-02-01T14:07:02Z"
  name: tenant1-prometheus-config-map
  namespace: tenant1
  ownerReferences:
  - apiVersion: minio.min.io/v2
    blockOwnerDeletion: true
    controller: true
    kind: Tenant
    name: tenant1
    uid: 0a5c90aa-44a7-4ec9-80b1-e2236ae0b35d
  resourceVersion: "160755"
  uid: e2aea3cb-7119-42e0-b4eb-5314220bb8d1

Yes thanks @kanagarajkm this is what I was chasing.

@harshavardhana
Copy link
Member Author

Everything should be fixed now @kanagarajkm @dvaldivia

Alevsk
Alevsk approved these changes Feb 1, 2022
View reviewed changes
kubectl-minio/cmd/proxy.go
Comment on lines +160 to +161
// outStr, errStr := string(stdout), string(stderr)
// fmt.Printf("\nout:\n%s\nerr:\n%s\n", outStr, errStr)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can delete commented code

pkg/controller/cluster/http_handlers.go
@@ -236,7 +234,7 @@ func (c *Controller) CRDConversionHandler(w http.ResponseWriter, r *http.Request
case "v1":

// cast to v1
//tenantV1 := obj.Object.(*miniov1.Tenant)
// tenantV1 := obj.Object.(*miniov1.Tenant)
Copy link
Contributor

Choose a reason for hiding this comment

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

delete commented code

pkg/controller/cluster/main-controller.go
@@ -792,7 +790,7 @@ func (c *Controller) syncHandler(key string) error {
}

// Check if this is fresh setup not an expansion.
//addingNewPool := len(tenant.Spec.Pools) == len(tenant.Status.Pools)
// addingNewPool := len(tenant.Spec.Pools) == len(tenant.Status.Pools)
Copy link
Contributor

Choose a reason for hiding this comment

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

delete commented code

@dvaldivia dvaldivia merged commit afb70fa into minio:master Feb 1, 2022
@harshavardhana harshavardhana deleted the move-goyaml-instead branch February 1, 2022 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alevsk Alevsk Alevsk approved these changes

@dvaldivia dvaldivia dvaldivia approved these changes

@adfost adfost Awaiting requested review from adfost

@sinhaashish sinhaashish Awaiting requested review from sinhaashish

@cniackz cniackz Awaiting requested review from cniackz

@kanagarajkm kanagarajkm Awaiting requested review from kanagarajkm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@harshavardhana @kanagarajkm @Alevsk @dvaldivia