text:
Validate OUI theme compliance - phase 1

This is a tracking issue for phase 1 of OUI theme plugin compliance, which will involve changes to OUI typography and color SASS variables (for both dark and light mode). For full details, including dates and tooling, see https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4291
If your plugin has no UI views or components, please add a comment to that effect, and we'll remove the repo from future look and feel update campaigns.
Actions required

[x] Acknowledge the issue by adding an assignee who will serve as a point-of-contact
[ ] Provide screenshots and navigation instructions for important views and screens in your plugin or application. This will assist with both manual and automatic validation of compliance
[ ] Identify all typographic properties and styles
[ ] Create issues to mitigate typographic styles not inherited from OUI components or SASS variables
[ ] Identify all color values, including visualization colors and palettes
[ ] Create issues to mitigate typographic styles not inherited from OUI components or SASS variables
[ ] Resolve all mitigation issues
[ ] Validate and test plugin with updated OUI theme

Questions?
Add a comment on https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4291

Hi @joshuarrrr there are mainly two kind of screens in the admin UI now. Here are the screenshots:

Deployed table list

ML node status panel

We will try to find out all the typographic and styles that not inherited from OUI components, and then try to mitigate them.

@wanglam To answer your offline question, for:
https://github.com/opensearch-project/ml-commons-dashboards/blob/e2825c48248311bf8299c9e988d2834635aadffc/public/components/preview_panel/index.tsx#L100
and
https://github.com/opensearch-project/ml-commons-dashboards/blob/e2825c48248311bf8299c9e988d2834635aadffc/public/components/preview_panel/index.tsx#L104
The inline-styling of the font size should just be removed, and use the default sizing from EuiDescriptionListTitle: https://oui.opensearch.org/1.0/#/display/description-list
cc @KrooshalUX for visibility of guidance.

@wanglam Can you provide some instructions or pointers to sample data and models? We'd like to be able to load that into our testing endpoint and reproduce the views you've included here.

Hi @joshuarrrr , Thank you for answering my questions. There is a model upload process in functional test. We can upload all the pre-trained models by this flow.
Here is a pure API flow to deploy a model with opensearch ml-commons 2.8 installed:

Enable ml-commons setting

PUT /_cluster/settings
{
  "transient": {
    "plugins.ml_commons.only_run_on_ml_node": false,
    "plugins.ml_commons.native_memory_threshold": 100
  }
}

Register model group

POST /_plugins/_ml/model_groups/_register
{
  "name":"test-model-group",
  "model_access_mode":"public"
}

Response example
{
  "model_group_id": "jL7Z24gBBvRGm7DQ4Ip1",
  "status": "CREATED"
}

Upload pre-trained model with model group id

POST /_plugins/_ml/models/_upload
{
  "name": "huggingface/sentence-transformers/all-MiniLM-L12-v2",
  "version": "1.0.1",
  "model_group_id": "jL7Z24gBBvRGm7DQ4Ip1",
  "model_format": "TORCH_SCRIPT"
}

Response example
{
  "task_id": "n77b24gBBvRGm7DQMIrf",
  "status": "CREATED"
}

Get model id by task id

GET /_plugins/_ml/tasks/n77b24gBBvRGm7DQMIrf

Response example
{
  "model_id": "l_Hb24gBXxK7xf32MUso",
  "task_type": "DEPLOY_MODEL",
  "function_name": "TEXT_EMBEDDING",
  "state": "COMPLETED",
  "worker_node": [
    "znWMK6yUT7mlFUTVP5SOLQ"
  ],
  "create_time": 1687315755231,
  "last_update_time": 1687315763626,
  "is_async": true
}

The model id won't be exists if task state not be COMPLETED, maybe we need to call get task API for multi times.

Deploy model with model id

POST /_plugins/_ml/models/l_Hb24gBXxK7xf32MUso/_deploy

Response example
{
  "task_id": "w77d24gBBvRGm7DQ94pR",
  "status": "CREATED"
}

We can use the get task API to track the deploy process, the model will be list in admin UI once deploy success.

@joshuarrrr There are another customize styles in the repo. Here are the codes:

customize fontWeight

Not responding on {planningNodesCount} of{' '}

customized backgroundColor

There are some style properties includes padding, width and maxWidth. Do we need to mitigate these?
There are some style value in the scss file, is that means we need to change to oui variables?

@AMoo-Miki When you get a chance, can you confirm the pre-trained model instructions  in https://github.com/opensearch-project/ml-commons-dashboards/issues/217#issuecomment-1599996062 are sufficient

customize fontWeight
Not responding on {planningNodesCount} of{' '}

In general, we shouldn't be manually specifying font-weight. 600 corresponds to the $ouiFontWeightSemiBold SASS variable: https://github.com/opensearch-project/oui/blob/a0e0bb5dc3f6787d4f2872da72c57a80a7cafd6c/src/global_styling/variables/_typography.scss#L67
In this case, I'd recommend replacing the  tag with a vanilla  tag.

customized backgroundColor

This should be replaced with an OUI component that sets the correct background automatically. I can review with @KrooshalUX and provide specific guidance.

It would be helpful to understand why the optimizations were being made - in some cases it may be because the component was detached in Figma therefore exposing exact values instead of the OUI Sass values. In some cases it may be because the designs that were handed off had minor deviations from standards, but even in this case, the standards should always be used from OUI. I believe that @lauralexis  had delivered some of these mockups prior to our internal Figma library launch, which may have lead engineering to believe optimizations should have been made.
@wanglam if you can help outline that, @lauralexis and I can provide further guidance.

@KrooshalUX @lauralexis The duration picker isn't an OUI component. The background of the duration picker is white just like below image.

Since the input is read-only, the default background of FieldText is rgba(211, 218, 230, 0.05), the UI will like below image.

Could we just remove the white background and use the default FieldText readonly background?

@wanglam This is in regards to the auto refresh feature correct? I think I recall trying to work through this with @lauralexis during the design phase.
I am aware that the current ReadOnly state for our form controls is lacking in contrast between the active state of the form control, and perhaps that is why this customization was made to SuperDatePicker. I am wondering if instead we need to have the SuperDatePicker functionality next to an on/off switch instead?

@KrooshalUX The UI of SuperDatePicker isn't 100% the same as our component. There is a min refresh rate (10 seconds) in our plugin. We will show a error tip if user input a value less than 10 seconds.

The start button will be disabled and the tip will turn red after less than 10 seconds. Seems we can't do these by using SuperDatePicker. Could you give me more suggestions about that?

@joshuarrrr We have some customized color in the scss files. Is that means we need to replace it with oui variables.
Here are the codes:

https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/monitoring/index.scss#L18
https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/preview_panel/nodes_table.scss#L4

@joshuarrrr We have some customized color in the scss files. Is that means we need to replace it with oui variables? Here are the codes:
* https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/monitoring/index.scss#L18

* https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/preview_panel/nodes_table.scss#L4

Yeah, you'll definitely want to update both of those to use the OUI SASS variables. @lauralexis can you provide the corret variable?

@joshuarrrr @wanglam
The following two should be: $euiLinkColor

https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/monitoring/index.scss#L18

https://github.com/opensearch-project/ml-commons-dashboards/blob/main/public/components/preview_panel/nodes_table.scss#L4

@wanglam Got it, thanks for the context refresher - its been a while since I have looked at the UI. All good to keep the higher order component you have built, please ignore my question about switching to SuperDatePicker.

@joshuarrrr I've created this PR to mitigate all the typographic and color styles. Would you mind to help me review it?

authors: KrooshalUX joshuarrrr lauralexis wanglam


text:
[Feature Request] Add support for LiteLLM

Is your feature request related to a problem? Please describe
Hi admin
I have commented here: https://github.com/Filimoa/open-parse/issues/10#issuecomment-2214343987, but after that, I found here could be a better place to put this comment:
I would like to suggest adding support for Litellm.
Litellm is an open source project, which unifies the API call of 100+ LLMs (including Anthropic, Cohere, and Ollama, etc) in an OpenAI compatible format: https://github.com/BerriAI/litellm
I believe integrating Litellm would be a fantastic enhancement because people could choose to switch or use their preferred embedding model api instead of OpenAI's ones only when dealing with semantic processing. Thanks.
Describe the solution you'd like
For example, if they used litellm python client and without self hosting litellm proxy, then their code could be like this (which is very consistent with OpenAI python client format):

Reference: https://github.com/BerriAI/litellm
if someone self hosted litellm proxy, which they can call LLM API in an OpenAI compatible format via llmlite proxy, you could see the code could be as follows:

Reference: https://litellm.vercel.app/docs/providers/azure_ai#passing-additional-params---max_tokens-temperature
You can see if someone self host the litellm proxy, he will only need to change the openai's base url, and all the other code are similar as openai's one...
Reference: https://litellm.vercel.app/docs/proxy/user_keys
import openai
client = openai.OpenAI(
    api_key="anything",
    base_url="http://0.0.0.0:4000"
)

# request sent to model set on litellm proxy, `litellm --model`
response = client.chat.completions.create(
    model="gpt-3.5-turbo",
    messages = [
        {
            "role": "user",
            "content": "this is a test request, write a short poem"
        }
    ],
    extra_body={ # pass in any provider-specific param, if not supported by openai, https://docs.litellm.ai/docs/completion/input#provider-specific-params
        "metadata": { # 👈 use for logging additional params (e.g. to langfuse)
            "generation_name": "ishaan-generation-openai-client",
            "generation_id": "openai-client-gen-id22",
            "trace_id": "openai-client-trace-id22",
            "trace_user_id": "openai-client-user-id2"
        }
    }
)

print(response)

There are also quite a few projects that used litellm: https://litellm.vercel.app/docs/project to call models from different providers on LiteLLM.
Hope for consideration, thanks.
Related component
Libraries
Describe alternatives you've considered
No response
Additional context
No response

[Catch All Triage - 1, 2, 3]

source: gharchive/issue

authors: dblock tan-yong-sheng

repo: opensearch-project/ml-commons

license_type: permissive

license_source: github-api


text:
[RFC] Asynchronous Offline Batch Inference and Ingestion to OpenSearch

Problem Statement
Nowadays remote model servers like AWS SageMaker, BedRock, or OpenAI, Cohere, etc all support batch predict APIs, which allow users to send large amount of synchronous requests in a file like S3 and produce the results asynchronously into a file as the output. Different platform use different terminology, e.g. SageMaker uses "batch transform" and bedrock uses "batch inference", but they are all the same in the sense of accepting the requests in the batch mode and process them asynchronously. While some uses require to send synchronous requests, but there are many cases where requests do not need an immediate response or certain rate limits prevent you from executing a large number of queries quickly. For example, in this case, the customer experiences throttling issues by the remote model and the data ingestion cannot finish.
In this RFC, "batch inference" is used as the terminology to represent the "batch" operations in all remote servers. The benefits of utilizing batch inference can be summaries but not limited to 1) Better cost efficiency: 50% cost discount compared to synchronous APIs (number may vary for different servers) and 2) Higher rate limits compared to the synchronous APIs.
For OpenSearch users, the most common use case of batch inference is to ingest embedding data into the K-NN index for vector search. However, the entire Ingest API in OpenSearch was designed for stream data processing. So all of the data processing capabilities supported by our ingestion processors aren’t optimized for batch ingestion and they do not accept files as inputs for ingestion.
This RFC focus on the these two improvements:

Enhance our AI connector framework in order to support these batch APIs
Provide a new ingest API to ingest data into the OpenSearch from files like S3, openAI files, etc which are the output of these batch APIs.

Proposed Solution

Phase 1 : Add new “batch_predict” action type in AI Connector framework (released in OpenSearch 2.16)
Add a new action type “batch_predict” in the connector blueprint. This action type is to run batch prediction using the connector in ML-Commons. An example of the connector for sageMaker is given below. Different model frameworks have different configuration of the connector, and we will define multiple ConnectorExecutors to easily extend batch prediction for other new AI models.
# Example API requestPOST /_plugins/_ml/connectors/_create
POST /_plugins/_ml/connectors/_create
{
  "name": "DJL Sagemaker Connector: all-MiniLM-L6-v2",
  "version": "1",
  "description": "The connector to sagemaker embedding model all-MiniLM-L6-v2",
  "protocol": "aws_sigv4",
  "credential": {
    "access_key": "<your access key>",
    "secret_key": "<your secret key>",
    "session_token": "<your session token>"
  },
  "parameters": {
    "region": "us-east-1",
    "service_name": "sagemaker",
    "DataProcessing": {
        "InputFilter": "$.content",
        "JoinSource": "Input",
        "OutputFilter": "$"
    },
    "ModelName": "DJL-Text-Embedding-Model-imageforjsonlines",
    "TransformInput": {
      "ContentType": "application/json",
      "DataSource": {
         "S3DataSource": {
            "S3DataType": "S3Prefix",
            "S3Uri": "s3://offlinebatch/sagemaker_djl_batch_input.json"
         }
      },
      "SplitType": "Line"
    },
    "TransformJobName": "SM-offline-batch-transform-07-12-13-30",
    "TransformOutput": {
      "AssembleWith": "Line",
      "Accept": "application/json",
      "S3OutputPath": "s3://offlinebatch/output"
   },
   "TransformResources": {
      "InstanceCount": 1,
      "InstanceType": "ml.c5.xlarge"
   },
   "BatchStrategy": "SingleRecord"
  },
  "actions": [
    {
      "action_type": "predict",
      "method": "POST",
      "headers": {
        "content-type": "application/json"
      },
      "url": "https://runtime.sagemaker.us-east-1.amazonaws.com/endpoints/OpenSearch-sagemaker-060124023703/invocations",
      "request_body": "${parameters.input}",
      "pre_process_function": "connector.pre_process.default.embedding",
      "post_process_function": "connector.post_process.default.embedding"
    },
    {
        "action_type": "batch_predict",
        "method": "POST",
        "headers": {
            "content-type": "application/json"
        },
        "url": "https://api.sagemaker.us-east-1.amazonaws.com/CreateTransformJob",
        "request_body": "{ \"BatchStrategy\": \"${parameters.BatchStrategy}\", \"ModelName\": \"${parameters.ModelName}\", \"DataProcessing\" : ${parameters.DataProcessing}, \"TransformInput\": ${parameters.TransformInput}, \"TransformJobName\" : \"${parameters.TransformJobName}\", \"TransformOutput\" : ${parameters.TransformOutput}, \"TransformResources\" : ${parameters.TransformResources}}"
    }
  ]
}

After the connector new action type is added, add a new API for batch inference jobs. To be consistent with the current “Predict” API in ML-Commons, the new API is named batch-prediction-job. This API maps to the “create_transform_job” API in SageMaker and “model-invocation-job” API in Bedrock. Example request of the new batch inference API.
POST /_plugins/_ml/models/dBK3t5ABrxVhHgFYhg7Q/_batch_predict
{
  "parameters": {
    "TransformJobName": "SM-offline-batch-transform-07-15-11-30"
  }
}
# Response
{
   "job_arn": "arn:aws:sagemaker:us-east-1:802041417063:transform-job/SM-offline-batch-transform"
}
{
    'task_id': 'xxxxxxxx',
    "state": "[InProgress]
}

Phase 2 : Add offline batch inference task management and new batch ingestion API.
Add the task management for the batch transform jobs which maps to the “list-prediction-jobs”, “describe-prediction-job” and “ cancel-prediction-job” etc APIs from remote model servers.
Add a new API for offline batch ingestion jobs. This API reads the batch inference results in the phase 1 and other input files, and bulk ingest the vectors into KNN index. This API reads the vector data from the S3 file, and organize the bulk ingestion request based on the field mapping. We will call the bulk ingestion to the KNN index asynchronously so a task id will be returned.
POST /_plugins/_ml/_batch_ingestion
{
  "index_name": "my-nlp-index",
  "field_map": {
    "chapter": "$.content[0]",
    "title": "$.content[1]",
    "chapter_embedding": "$.SageMakerOutput[0]"
     "title_embedding": "$.SageMakerOutput[1]",
    "ingest_fields": ["$.id"]
  },
  "credential": {
    "region": "us-east-1",
    "access_key": "<your access key>",
    "secret_key": "<your secret key>",
    "session_token": "<your session token>"
  },
  "data_source": {
    "type": "s3",
    "source": ["s3://offlinebatch/output/sagemaker_djl_batch_input.json.out"]
  }
}

# expected response
{
  "task_id": "cbsPlpEBMHcagzGbOQOx",
  "task_type": "BATCH_INGEST",
  "status": "CREATED"
}

Phase 3: Integrate the offline batch inference and ingestion engine in the data prepper
Data Prepper is a server-side data collector capable of filtering, enriching, transforming, normalizing, and aggregating data for downstream analytics and visualization. Data Prepper lets users build custom pipelines to improve the operational view of applications. We can integrate the AI connector in the data prepper for customer to setup custom pipelines to run offline batch ingestion.
A sample data Prepper config is presented below. Integration with data prepper can be implemented after phase 1 and 2. That usually means users have the option to use data Prepper to setup offline batch ingestion without directly calling any Plugins.
sample-pipeline:
  workers: 4 #Number of workers
  delay: 100 # in milliseconds, how often the workers should run
  source:
    file:
        path: <path/to/input-file> -> input file S3
  buffer:
    bounded_blocking:
      buffer_size: 1024 # max number of events the buffer will accept
      batch_size: 256 # max number of events the buffer will drain for each read
  processor:
    - string_converter:
       upper_case: true
    - text_embedding:
       model_id: <xxxxx> -> the model in ml-commons for batch inference
  sink:
    - file:
       path: <path/to/output-file> -> ourput file S3

In Scope

Offline Batch Inference is released in OpenSearch 2.16
Offline Batch Ingestion is scheduled to be released in OpenSearch 2.17
Offline Batch Inference and Ingestion Task/Job management is scheduled to be released in OpenSearch 2.17
The Integration of Offline Batch Inference with OSI is in the planning

Out of Scope

Integrate the offline batch engine into the OpenSearch Ingest processor and pipeline - Not in planning
Integrate the offline batch engine with other remote ingestion engine like Apache Airflow, Apache Spark (Flint). These external engines can be used for offline batch. This will be discussed in a separate RFC if prioritized.

Real Case Example using a public ML Model Service
Given an input file for batch inference using the OpenAI embedding model "text-embedding-ada-002":
{"custom_id": "request-1", "method": "POST", "url": "/v1/embeddings", "body": {"model": "text-embedding-ada-002", "input": [ "What is the meaning of life?", "The food was delicious and the waiter..."]}}
{"custom_id": "request-2", "method": "POST", "url": "/v1/embeddings", "body": {"model": "text-embedding-ada-002", "input": [ "What is the meaning of work?", "The travel was fantastic and the view..."]}}
{"custom_id": "request-3", "method": "POST", "url": "/v1/embeddings", "body": {"model": "text-embedding-ada-002", "input": [ "What is the meaning of friend?", "The old friend was far away and the time..."]}}
... ...

Use the connector below to kick off the batch inference through OpenAI batch API
POST /_plugins/_ml/connectors/_create
{
  "name": "OpenAI Embedding model",
  "description": "OpenAI embedding model for offline batch",
  "version": "1",
  "protocol": "http",
  "parameters": {
    "model": "text-embedding-ada-002",
    "input_file_id": "<your input file id in OpenAI>",
    "endpoint": "/v1/embeddings"
  },
  "credential": {
    "openAI_key": "<your key>"
  },
  "actions": [
    {
         ...
    },
    {
      "action_type": "batch_predict",
      "method": "POST",
      "url": "https://api.openai.com/v1/batches",
      "headers": {"Authorization": "Bearer ${credential.openAI_key}"},
      "request_body": "{ \"input_file_id\": \"${parameters.input_file_id}\", \"endpoint\": \"${parameters.endpoint}\", \"completion_window\": \"24h\" }"
    },
    {
      "action_type": "batch_predict_status",
      "method": "GET",
      "url": "https://api.openai.com/v1/batches/${parameters.id}",
      "headers": {
        "Authorization": "Bearer ${credential.openAI_key}"
      }
    }
  ]
}

# Invoke the model with "batch_predict" action type
POST /_plugins/_ml/models/<your model associated with the connector>/_batch_predict
{
  "parameters": {
    "input_file_id": "<your file id>"
  }
}

# Response
{
 "task_id": "KYZSv5EBqL2d0mFvs80C",
 "status": "CREATED"
}

Once you receive the task_id, you can check the task through the Task API to check the remote job status in OpenAI.
// Get batch job status using get task API
GET /_plugins/_ml/tasks/KYZSv5EBqL2d0mFvs80C

// Response
{
 "model_id": "JYZRv5EBqL2d0mFvKs1E",
 "task_type": "BATCH_PREDICTION",
 "function_name": "REMOTE",
 "state": "COMPLETED",
 "input_type": "REMOTE",
 "worker_node": [
 "Ee5OCIq0RAy05hqQsNI1rg"
 ],
 "create_time": 1725491751455,
 "last_update_time": 1725491751455,
 "is_async": false,
 "remote_job": {
 "cancelled_at": null,
 "metadata": null,
 "request_counts": {
 "total": 3.0,
 "completed": 3.0,
 "failed": 0.0
 },
 "input_file_id": "file-5gXEtbKjHnYrKrdtv69IeRN2",
 "output_file_id": "file-Wux0Pk80dhkxi98Z5iKNjB4n",
 "error_file_id": null,
 "created_at": 1.725491753E9,
 "in_progress_at": 1.725491753E9,
 "expired_at": null,
 "finalizing_at": 1.725491757E9,
 "completed_at": 1.725491758E9,
 "endpoint": "/v1/embeddings",
 "expires_at": 1.725578153E9,
 "cancelling_at": null,
 "completion_window": "24h",
 "id": "batch_yz4YzfPfajDgcWFk4CqfxYox",
 "failed_at": null,
 "errors": null,
 "object": "batch",
 "status": "completed"
 }
}

After the job status is showing "completed", you can double check your output in the file id "file-Wux0Pk80dhkxi98Z5iKNjB4n" in this example. It's a file in the jsonl format that each line in the file represents a single inference result for a request in the input file.
The output file content is:
{"id": "batch_req_ITKQn29igorXCAGp6wzYs5IS", "custom_id": "request-1", "response": {"status_code": 200, "request_id": "10845755592510080d13054c3776aef4", "body": {"object": "list", "data": [{"object": "embedding", "index": 0, "embedding": [0.0044326545, ... ...]}, {"object": "embedding", "index": 1, "embedding": [0.002297497, ... ... ]}], "model": "text-embedding-ada-002", "usage": {"prompt_tokens": 15, "total_tokens": 15}}}, "error": null}
...
...

To ingest your embedding data and your other fields into OpenSearch, you can use the batch ingestion API given below. The field map is the actual fields you want to ingest in your KNN cluster. In the field map, the key is the field name, and the value is the JsonPath to find your data in the sources files. For example, source[1].$.body.input[1] means you want to use the JsonPath $.body.input[1] to fetch the element body.input[1] in the second file in the "source" array of the batch request.
POST /_plugins/_ml/_batch_ingestion
{
  "index_name": "my-nlp-index-openai",
  "field_map": {
    "question": "source[1].$.body.input[0]",
    "answer": "source[1].$.body.input[1]",
    "question_embedding":"source[0].$.response.body.data[0].embedding",
    "answer_embedding":"source[0].$.response.body.data[1].embedding",
    "_id": ["source[0].$.custom_id", "source[1].$.custom_id"],
  },
  "ingest_fields": ["source[2].$.custom_field1", "source[2].$.custom_field2", "source[2].$.custom_field3" ],
  "credential": {
    "openAI_key": "<your openAI key>"
  },
  "data_source": {
    "type": "openAI",
    "source": ["<your batch inference output file id>", "<your batch inference input file id>", "<your file id that contains other fields data>"]
  }
}

# expected response
{
  "task_id": "b5Yym5EBdxfqV0JWKrT6",
  "task_type": "BATCH_INGEST",
  "status": "CREATED"
}

# Once you obtain the task_id, use it to check the job status
GET /_plugins/_ml/tasks/cbsPlpEBMHcagzGbOQOx

# expected response, "state": "COMPLETED" means this task is successful
{
  "task_type": "BATCH_INGEST",
  "state": "COMPLETED",
  "create_time": 1724885641978,
  "last_update_time": 1724885642658,
  "is_async": true
}

Request for Comments
This Offline batch engine is implemented in the machine learning space. It's designed to incorporate with the remote Model Batch APIs to facilitate the batch inference of the LLM models and ingest ML model output data into the OpenSearch KNN index for vector search. However, the batch ingestion API can also be used for general ingestion purpose when the inputs are in a file system. So please not limit your thoughts only in the machine learning when you review this feature.
Please leave your suggestions and concerns in this RFC and your valuable inputs are appreciated.

@Zhangxunmt, I think it's important for us to continue supporting Ingest API compatibility. It's been in use since 2016, and we have major feature sets like neural search that are dependent on it.
Today, users can use the Ingest API to define an ingest pipeline.
PUT /_ingest/pipeline/my_pipeline
{
"description": "pipeline with inference processing",
"processors": [
{
{
"ml_inference": {
"model_id": "<model_id>",
"function_name": "<function_name>",
"full_response_path": "<full_response_path>",
"model_config":{
"<model_config_field>": "<config_value>"
},
"model_input": "<model_input>",
"input_map": [
{
"<model_input_field>": "<document_field>"
}
],
"output_map": [
{
"<new_document_field>": "<model_output_field>"
}
],
"override": ""
}
}
They can run a bulk streaming ingestion job like my-index/_bulk?pipeline=my-pipeline .
The user should be able to execute this pipeline in batch mode using a command like my-index/_batch?pipeline=my-pipeline source="s3://..."
Batch processing support for each ingestion processor can be incrementally added.

authors: Zhangxunmt dylan-tong-aws


text:
json listing of all the pretrianed models

Description
[json listing of all the pretrianed models]
Issues Resolved
[List any issues this PR will resolve]
Check List

[ ] New functionality includes testing.

[ ] All tests pass

[ ] New functionality has been documented.

[ ] New functionality has javadoc added

[ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

Merging #730 (ca66766) into 2.x (ffb8a4e) will decrease coverage by 0.06%.
The diff coverage is n/a.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
@@             Coverage Diff              @@
##                2.x     #730      +/-   ##
============================================
- Coverage     84.95%   84.90%   -0.06%
+ Complexity     1076     1074       -2
============================================
  Files           100      100
  Lines          3922     3922
  Branches        370      370
============================================
- Hits           3332     3330       -2
- Misses          433      435       +2
  Partials        157      157

Flag
Coverage Δ

ml-commons
84.90% <ø> (-0.06%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

...va/org/opensearch/ml/task/MLExecuteTaskRunner.java
76.66% <0.00%> (-6.67%)
:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

source: gharchive/pull-request

authors: codecov-commenter dhrubo-os


text:
Remove unnecessary trailing slashes.

Description
Coming from https://github.com/opensearch-project/opensearch-api-specification/pull/179 fixing some unnecessary trailing slashes. Other places were https://github.com/opensearch-project/security/pull/3976 and https://github.com/opensearch-project/OpenSearch/pull/11992.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@nhtruong Can we get this merged please?


text:
[Enhancement] Improve tag creation pipeline to skip repositories without write permission for the bot account

As of now, the release tag creation will fail the build if the bot account does not have permission to write in any repository.
This requires our team member with access to manually create a tag on those repos.
We should let the pipeline skip these repos, and output a list of repos that requires to add the bot with write access, and we can manually create the tags after the release pipeline run.
Thanks.

I think it's a bad idea. Permissions should be fixed anyway, and the workflow can be rerun as many times as needed as it skips correct existing tags. If you stop failing it, we'll never know that permissions are missing.

We are not going to stop failing rather we dont want the process to fail in the middle of the tagging process since it doesn't have access to those repos. The job will still fail listing the repos that has issue but it wont affect the other repos not affected due to access issues.

If the pipeline failed, we should always rerun the workflow to make sure tags are successfully created on other components until the job passes.
One drawback of current workflow I think is that we will need to rerun the job more than 2 times if there are multiple repos that bot doesn't have write access on.

[triage] We would want to create tags for all repositories, if creation fails, the repository should be skipped and continue for the remaining repositories. The job should then fail listing down the failed repositories.

Closing this issue as this enhancement have been implemented with PR linked above.

authors: abhinavGupta16 bbarani dblock peterzhuamazon zelinh

repo: opensearch-project/opensearch-build


text:
Updated dev guide with the correct GET route.

Description
Fixed up dev guide, notably using the correct GET route for the hello world request.
Added a JSON configuration file to avoid POSTing the entire JSON body when registering the extension.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock we have spotless for MD files as well :stuck_out_tongue:.
./gradlew :spotlessApply  to fix the spotless failures.

repo: opensearch-project/opensearch-sdk-java


text:
Incremented version to 1.2.3.

Coming from https://github.com/opensearch-project/opensearch-build/issues/1365.

Codecov Report

Merging #106 (63ce585) into 1.2 (725be0b) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##                1.2     #106   +/-   ##
=========================================
  Coverage     72.19%   72.19%
  Complexity      356      356
=========================================
  Files            44       44
  Lines          2510     2510
  Branches        160      160
=========================================
  Hits           1812     1812
  Misses          592      592
  Partials        106      106

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 725be0b...63ce585. Read the comment docs.

authors: codecov-commenter dblock

repo: opensearch-project/performance-analyzer


text:
Backport/backport 204/208 to 2.0

Is your feature request related to a problem? Please provide an existing Issue # , or describe.
#203 #204 #208
Check List

[x] New functionality includes testing.

[x] All tests pass

[ ] New functionality has been documented.

[ ] New functionality has javadoc added

[x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

Merging #216 (9eaad16) into 2.0 (f8e42d5) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                2.0     #216      +/-   ##
============================================
- Coverage     71.77%   71.68%   -0.09%
  Complexity      367      367
============================================
  Files            44       44
  Lines          2519     2522       +3
  Branches        169      171       +2
============================================
  Hits           1808     1808
- Misses          598      601       +3
  Partials        113      113

Impacted Files
Coverage Δ

...on/config/PerformanceAnalyzerResourceProvider.java
35.71% <0.00%> (-1.13%)
:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8e42d5...9eaad16. Read the comment docs.

authors: codecov-commenter jainankitk


text:
Update main and 2.x branches to comply with the OpenSearch plugin development standards

Is your feature request related to a problem?
We need to get release branches comply with the OpenSearch plugin development standards
What solution would you like?
After the plugin repo is set up with initial commits, we need to get main and releaes branches comply with the OpenSearch plugin development standards, more specifically, we need to set the OpenSearch version in main to 3.0.0-SNAPSHOT and cut the 2.x branch.

[Catch All Triage, attendees 1, 2, 3, 4, 5, 6, 7]

For 2.18 release, we also need to add query insights dashboards into the manifest file like: https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.17.0/opensearch-dashboards-2.17.0.yml

authors: ansjcy dblock


text:
Add link to the GitHub Release Planning project

Action item from the face to face

LGTM

LGTM

LGTM

LGTM

authors: angarg12 avade duglin mattmcneeney pmorie

repo: openservicebrokerapi/servicebroker

license_source: bigquery


text:
Track Jenkins job run on alignment PRs

Needed by https://github.com/openshift-eng/art-tools/pull/267

/lgtm

authors: locriandev mbiarnes

repo: openshift-eng/aos-cd-jobs


text:
[release-v1.15] Bump to latest upstream reconciler-test

updated go.mod to pin to latest from upstream 1.15
ran make generate-release to consume the updated reconciler test.
However, afterwards I also had to manually apply these two patches:
git apply openshift/patches/020-mutemetrics.patch && git apply openshift/patches/027-rekt-serviceaccounts-delete.patch

Not sure why they are ignored:
https://github.com/openshift-knative/eventing/blob/release-v1.15/openshift/release/generate-release.sh#L15-L16

/assign @pierDipi

repo: openshift-knative/eventing


text:
[release-v1.15] fix: add case for DNS resolve fail in TestDialUnreachable

Backport of https://github.com/knative/func/pull/2619/files

/cherry-pick release-v1.16

:warning: Please install the  to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (release-v1.15@6242c15). Learn more about missing BASE report.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@               Coverage Diff                @@
##             release-v1.15    #1017   +/-   ##
================================================
  Coverage                 ?   64.99%
================================================
  Files                    ?      129
  Lines                    ?    15408
  Branches                 ?        0
================================================
  Hits                     ?    10014
  Misses                   ?     4423
  Partials                 ?      971

Flag
Coverage Δ

e2e-test
36.86% <ø> (?)

e2e-test-runtime-go
26.49% <ø> (?)

e2e-test-runtime-node
25.89% <ø> (?)

e2e-test-runtime-python
25.89% <ø> (?)

e2e-test-runtime-quarkus
26.04% <ø> (?)

e2e-test-runtime-rust
25.05% <ø> (?)

e2e-test-runtime-springboot
25.09% <ø> (?)

e2e-test-runtime-typescript
26.00% <ø> (?)

integration-tests
51.88% <ø> (?)

unit-tests
50.61% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

/approve
/lgtm

/retest-required

/override "Red Hat Konflux / serverless-operator-135-ec / kn-plugin-func-func-util-115"

/override ci/prow/417-e2e-oncluster-test
/override ci/prow/417-images
/override ci/prow/417-unit

authors: codecov-commenter creydr matejvasek

repo: openshift-knative/kn-plugin-func


text:
MGMT-16066: Resource Subscription Server

Description:

update the alarm subscription handler to a generic subscription handler that can also be used for resource subscriptions
add a new command for the resource subscription server:
oran-o2ims start infrastructure-inventory-subscription-server
update the alarm and resource subscription servers to also set the subscription type

Looks good to me.

/lgtm

/approve

authors: danielerez irinamihai jhernand


text:
feat: optimize feature-toggles API spec

Context and reasoning behind this - https://redhat-internal.slack.com/archives/C045X0P531D/p1676582610217699

TODO: As per the new design in this PR, evaluate if the featureToggleHandler interface can be simplified

/test lint-and-precommit-check

/lgtm

/lgtm

/retest

repo: openshift/addon-operator


text:
[release-4.13] OCPBUGS-20964: CVE-2023-44487: bump golang.org/x/net to v0.17.0

cc @openshift/storage

/jira refresh

/label backport-risk-assessed

CI result looks good.
Verified with golang.org/x/net v0.17.0 in go.mod file
/label cherry-pick-approved
/label qe-approved

/jira refresh

/lgtm

authors: bertinatto dobsonj ropatil010


text:
NO-JIRA: Sync changes from release-1.15 branch

PR has following changes:

Sync common changes from release-1.15 branch like Makefile, renovate config, hack scripts..
Updates the tekton pipeline code with customization to avoid redundant builds, multi platforms config.

EC policy validation tests are failing because of below vulnerabilities reported in clair scan, which will be fixed in follow up PR.
			{
				"msg": "Found packages with critical vulnerabilities. Consider updating to a newer version of those packages, they may no longer be affected by the reported CVEs.",
				"metadata": {
					"details": {
						"description": "Vulnerabilities found: golang.org/x/crypto-v0.24.0 (GHSA-v778-237x-gjrc)",
						"name": "clair_critical_vulnerabilities",
						"url": "https://access.redhat.com/articles/red_hat_vulnerability_tutorial"
					},
					"vulnerabilities_number": 1
				}
			},
			{
				"msg": "Found packages with high vulnerabilities. Consider updating to a newer version of those packages, they may no longer be affected by the reported CVEs.",
				"metadata": {
					"details": {
						"description": "Vulnerabilities found: golang.org/x/net-v0.26.0 (GHSA-w32m-9786-jp63)",
						"name": "clair_high_vulnerabilities",
						"url": "https://access.redhat.com/articles/red_hat_vulnerability_tutorial"
					},
					"vulnerabilities_number": 1
				}
			},

/override "Red Hat Konflux / cert-manager-operator-1-14-enterprise-contract / cert-manager-operator-1-14"
/override "Red Hat Konflux / cert-manager-operator-1-14-enterprise-contract / cert-manager-operator-bundle-1-14"
/override "Red Hat Konflux / jetstack-cert-manager-1-14-enterprise-contract / jetstack-cert-manager-1-14"
/override "Red Hat Konflux / jetstack-cert-manager-1-14-enterprise-contract / jetstack-cert-manager-acmesolver-1-14"
/override "Red Hat Konflux / jetstack-cert-manager-1-14-enterprise-contract / pr group release-1.14"

/lgtm

authors: bharath-b-rh swghosh

repo: openshift/cert-manager-operator-release


text:
Read-only root file system

Add readOnlyRootFilesystem to our Deployments and StatefulSets. Why? Cause it's more securer.
One thing I needed to do to make this work was to add a volume for /tmp, which I guess is by default part of /. This resulted in an actual problem in hive-controllers creating a Session for AWS... but I did it for all the things just in case.
This scares me a little bit. For one thing, any time we mess with securityContexts we seem to break our friends running hive under vanilla k8s. But also, who knows what corner cases are expecting to be able to write somewhere in the root file system? We won't find out if, like, trying to use additional certs under OpenStack is a problem until some customer tries it.
HIVE-2350

/hold for testing and risk assessment
/test e2e-gcp
/test e2e-azure

/test security
just to make sure we didn't pick up any others before I fix this

Glad I checked. Resolving the two new vulns via HIVE-2398 / #2180.

/test security

/hold cancel
:crossed_fingers:

/override ci/prow/security
Same low-sev, not-yet-fixed vuln we've been tracking.

/cherry-pick mce-2.5

repo: openshift/hive


text:
Remove all liveness probes

What this PR does / why we need it:
Probes are expensive at scale and should have a justification for existing.
Liveness probes, in particular, are rarely justifiable and generally do more harm than good.  They load the kubelet, cause unneeded pod restarts, and rarely (never?) fix the problem as our components either retry or die on their own.
I would rather allocate our probe budget toward more frequent readiness probes on important services to improve the responsiveness of the control plane.
Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #
Checklist

[ ] Subject and description added to both, commit and PR.
[ ] Relevant issues have been referenced.
[ ] This change includes docs.
[ ] This change includes unit tests.

/hold
I am not sure if removing all livliness probes is an acceptable answer and impacts monitoring

Learned a few things looking into this

kube-controller-manager and kube-scheduler depend on the liveness probes to die if they lose the leader lease
https://github.com/kubernetes/client-go/blob/f6ce18ae578c8cca64d14ab9687824d9e1305a67/tools/leaderelection/healthzadaptor.go#L25-L31

all controller-runtime and library-go based operators exit on their own if they lose the leader lease

CPO, HCCO, and ignition-server all hardcode their healthz and readyz to true

@sjenning still pursuing this or can it be closed?

authors: enxebre relyt0925 sjenning

repo: openshift/hypershift


text:
OCPBUGS-34865: Add HTTP(s) konnectivity proxy for openshift-apiserver

What this PR does / why we need it:
Introduces a new command under cpo: konnectivity-https-proxy that
performs a similar job to konnectivity-socks-proxy except that it
exposes the http protocol. It allows passing user-configured proxy
configuration (http_proxy, https_proxy, no_proxy) as flag values and
uses them when tunneling traffic through konnectivity.
The new konnectivity HTTPS proxy allows setting the proxy to use for
data plane requests. This commit switches the proxy used by the
OpenShift APIServer to the new proxy and configures it with the proxy
configuration if specified in HostedCluster.spec.configuration.proxy
Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #OCPBUGS-34865
Checklist

[x] Subject and description added to both, commit and PR.
[x] Relevant issues have been referenced.
[ ] This change includes docs.
[ ] This change includes unit tests.

/jira refresh

/retest

/retest
/lgtm

/hold

/hold cancel

Thanks!
/lgtm

authors: csrwng enxebre jparrill


text:
Allow the KAS to finish its graceful shutdown

Currently, the KAS has a 30 seconds TerminationGracePeriodSeconds but
its shutdown sequence takes 90 seconds, where it still takes requests
for the first 30 seconds and finishes responding to them for the next 60
seconds. This is to ensure that client requests created during a rollout
still get a response.
The current setting means that the kubelet just kills the KAS after 30
seconds, interrupting this sequence. Bump TerminationGracePeriodSeconds
to 95 seconds to allow the shutdown sequence to finish. This is visible
in the KAS log where we now see [graceful-termination] apiserver is exiting in the very end after deleting a pod.
The shutdown sequence of the KAS is described in more detail here:
https://github.com/openshift/installer/blob/master/docs/dev/kube-apiserver-health-check.md

/lgtm

authors: alvaroaleman csrwng


text:
OLS-1160: Updated list of packages to prefetch

Description
Updated list of packages to prefetch
Type of change

[ ] Refactor
[ ] New feature
[ ] Bug fix
[ ] CVE fix
[ ] Optimization
[ ] Documentation Update
[ ] Configuration Update
[ ] Bump-up dependent library
[ ] Bump-up library or tool used for development (does not change the final image)
[ ] CI configuration change
[x] Konflux configuration change

Related Tickets & Documents

Related Issue #OLS-1160

/lgtm

/unhold

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.92%. Comparing base (b6cc49b) to head (861a1ae).
Report is 52 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1767      +/-   ##
==========================================
+ Coverage   94.90%   94.92%   +0.02%
==========================================
  Files          60       60
  Lines        2689     2701      +12
==========================================
+ Hits         2552     2564      +12
  Misses        137      137

/override "Red Hat Konflux / ols-enterprise-contract / lightspeed-service"

/hold

/unhold

authors: codecov-commenter raptorsun tisnik

repo: openshift/lightspeed-service


text:
Use lowercase local vars in LLM loader tests

Description
Use lowercase local vars in LLM loader tests
Why is this bad?
[PEP 8] recommends that all function variables use lowercase names:

Function names should be lowercase, with words separated by underscores as necessary to
improve readability. Variable names follow the same convention as function names. mixedCase
is allowed only in contexts where that's already the prevailing style (e.g. threading.py),
to retain backwards compatibility.

Type of change

[x] Refactor
[ ] New feature
[ ] Bug fix
[ ] CVE fix
[ ] Optimization
[ ] Documentation Update
[ ] Configuration Update
[ ] Bump-up dependent library

Checklist before requesting a review

[x] I have performed a self-review of my code.
[x] PR has passed all pre-merge test jobs.
[ ] If it is a core feature, I have added thorough tests.

Testing

N/A

/approve
needs a rebase

/lgtm

authors: bparees tisnik


text:
OCPBUGS-7056: fix ovn config file path

GetConfigFile() may return empty path or user defined path, but ovn config file is only supported under /etc/microshift
Signed-off-by: Zenghui Shi zshi@redhat.com

@zshi-redhat, can we link this PR to a JIRA issue/bug?

/jira refresh

/jira refresh

@zshi-redhat, can we link this PR to a JIRA issue/bug?

Done.

/lgtm

/retest

/cherrypick release-4.12

authors: ggiguash zshi-redhat

repo: openshift/microshift


text:
CNV-46603: Create project modal with ClusterUDN creation

Select ClusterUDN
Using this option we'll add the needed labels to the project to respect the ClusterUDN matchLabels.
(We can't edit the project directly but if we patch the namespace, the project will get the labels)

/hold

/unhold

/cherry-pick release-4.18

repo: openshift/networking-console-plugin


text:
OCS Functional Test Suite

Functional Tests
Our functional test suite uses the ginkgo testing framework.
Prerequisites for running Functional Tests

ocs must already be installed
KUBECONFIG env var must be set

Running functional test
make functest
Below is some sample output of what to expect.
Building functional tests
hack/build-functest.sh
GINKO binary found at /home/dvossel/go/bin/ginkgo
Compiling functests...
    compiled functests.test
Running functional test suite
hack/functest.sh
Running Functional Test Suite
Running Suite: Tests Suite
==========================
Random Seed: 1568299067
Will run 1 of 1 specs

•
Ran 1 of 1 Specs in 7.961 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Developing Functional Tests
All the functional test code lives in the functests/ directory. For an
example of how a functional test is structured, look at the functests/pvc_creation_test.go
file.
The tests themselves should invoke simple to understand steps. Put any complex
logic in the functests/utils.go package so test flows are easy to follow.
Running a single test
When developing a test, it's common to just want to run a single functional test
rather than the whole suite. This can be done using ginkgo's "focus" feature.
All you have to do is put a F in front of the tests declaration to force
only that test to run. So, if you have an iteration like It("some test")
defined, you just need to set that to FIt("some test") to force the test
suite to only execute that single test.
Make sure to remove the focus from your test before creating the pull request.
Otherwise the test suite will fail in CI.

I think @jarrpa's request to split up utils.go still remains. Otherwise looks good to me.

Yeah, i broke up the file a bit, but looking back at it I think I can do better. I'll make another update

I think the functests uncovered a possible issue. It looks like the StorageCluster is reporting that it is available before the osd's have started.

I think I traced the issue back to how the functional tests wait for the StorageCluster startup condition. If not all expected conditions existed in the list, it looked like it was possible for the "BeforeTestSuite" function to exit earlier than it should have.
I added logic to address this. We'll see if it improves the situation. I was able to verify that the StorageCluster's conditions were set correctly to indicate that it was still progressing when the OSDs were not online. So, it seems likely that what I saw was test related.

The latest CSV appears to be broken. The functional test failure looks real.
2019-09-13 17:01:13.574026 E | op-osd: failed to create provision job for node example-deviceset-0-jhkfl. Job.batch "rook-ceph-osd-prepare-example-deviceset-0-jhkfl" is invalid: [spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Required value: can not be empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must be non-empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')]
2019-09-13 17:01:13.811323 E | op-osd: failed to create provision job for node example-deviceset-1-bqqv6. Job.batch "rook-ceph-osd-prepare-example-deviceset-1-bqqv6" is invalid: [spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Required value: can not be empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must be non-empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')]
2019-09-13 17:01:15.818636 E | op-osd: failed to create provision job for node example-deviceset-2-6kv9t. Job.batch "rook-ceph-osd-prepare-example-deviceset-2-6kv9t" is invalid: [spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Required value: can not be empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must be non-empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')]

and
2019-09-13 17:01:59.350313 E | op-cluster: failed to create cluster in namespace openshift-storage. failed to start the osds. 3 failures encountered while running osds in namespace openshift-storage: failed to create provision job for node example-deviceset-0-jhkfl. Job.batch "rook-ceph-osd-prepare-example-deviceset-0-jhkfl" is invalid: [spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Required value: can not be empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must be non-empty, spec.template.spec.affinity.podAntiAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].podAffinityTerm.topologyKey: Invalid value: "": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')]
failed to create provision job for node example-deviceset-1-bqqv6. Job.batch "rook-ceph-osd-prepare-examp

rook-ceph can't start the osd pods. It seems likely this is some regression in the new rook-ceph image we introduced a few hours ago in this pr, https://github.com/openshift/ocs-operator/pull/121

How did the test pass on that PR then?

How did the test pass on that PR then?

I might have misunderstood the question. If you meant how did the e2e test pass on pr #121 it's because there are no functional tests being executed until this PR is merged. The e2e logic as it is without the functional test suite just verifies that the ocs-operator installs and that the operator is online, not that a storage cluster comes online. That's the part this PR adds, along with verifying PVC creation.

/test ocs-operator-e2e-aws

...ah, right. Okay, we'll probably have to wait until Monday to figure this out. Pinging @kshlm for awareness.

@jarrpa https://github.com/openshift/ocs-operator/pull/122 should fix this. We weren't setting the topologyKey in our antiaffinity rule that we use on the cephcluster. That causes spec to get rejected during validation by OCP4.

Gah! I don't know how I missed that one. Merged.
/retest

/hold

The tests fail waiting for OSD to come up. From the few test logs I saw, the failure happened just as osds were being created. So maybe bump up the timeout for OSDs.

The tests fail waiting for OSD to come up. From the few test logs I saw, the failure happened just as osds were being created. So maybe bump up the timeout for OSDs

I'll up the timeout for now so we can get this PR in.
Independently of this PR I'm going to circle back around and try to gain an understanding for why this is occurring though. It appears like the StorageCluster CR may be reporting that it's in a stable state (available, upgradable, not progressing, and not degrading) at some point before the OSDs actually come online, but the CR is clearly reflecting the correct status of the StorageCluster when the timeout occurs.

I bumped the setup timeout to 30 minutes, we'll see if this improves the situation.

I changed the logic to wait an entire hour for the OSDs to come online, and the e2e test is still timing out.
I'm trying to reproduce this locally using a similar cluster to what the e2e tests use. One difference I noticed is that they're only using two AZs, but after talking with @jarrpa it appears this theoretically this should be fine.

I was able to reproduce what's happening in Openshift CI by limiting my OCP deployment to only 2 availability zones in AWS.
Only 1 or 2 OSDs ever come online. The OSDs that don't come online complain about not being able to attach a PVC to the OSDs pod. Below is an example.
Events:
  Type     Reason              Age                 From                                   Message
  ----     ------              ----                ----                                   -------
  Normal   Scheduled           46m                 default-scheduler                      Successfully assigned openshift-storage/rook-ceph-osd-0-574bb9858b-kfltj to ip-10-0-129-254.ec2.internal
  Warning  FailedAttachVolume  46m                 attachdetach-controller                Multi-Attach error for volume "pvc-230b857f-d8af-11e9-b766-0e15af88a5cc" Volume is already used by pod(s) rook-ceph-osd-prepare-example-deviceset-2-khhxc-22t6n
  Warning  FailedMount         88s (x20 over 44m)  kubelet, ip-10-0-129-254.ec2.internal  Unable to mount volumes for pod "rook-ceph-osd-0-574bb9858b-kfltj_openshift-storage(43ee9141-d8af-11e9-bed1-0a89cbfd94e0)": timeout expired waiting for volumes to attach or mount for pod "openshift-storage"/"rook-ceph-osd-0-574bb9858b-kfltj". list of unmounted volumes=[example-deviceset-2-khhxc]. list of unattached volumes=[rook-data rook-config-override rook-ceph-log devices example-deviceset-2-khhxc example-deviceset-2-khhxc-bridge rook-binaries run-udev rook-ceph-osd-token-fcbk9]

In order to stabilize the functional tests so we can get at least something merged that verifies basic funcitonality, I'm experimenting with reducing the OSD replica count to 1 just for the functional test environment.

@jarrpa @obnoxxx Functional tests pass again. I had to deploy a work around for issue #131
The osd issue occurs when we have multiple worker nodes within the same AZ. Openshift CI gives us only 2 AZs and 3 worker nodes, which means we'll always have at least 1 extra node in an AZ. For the time being, in order to work around #131 I only label a single node from each AZ and limit the functional test to Replica 2.
Using Replica 2 for the e2e test suite should only be considered a stop gap. I'm tracking the root cause of the issue in #131 and I've also opened up a discussion with Openshift CI about increasing the AZ count to 3. https://github.com/openshift/release/pull/5047
Here's my suggestion for moving forward.

Merge this PR so we have an added level of confidence when developing ocs-operator changes.
Consider removing the e2e test as a mandatory test. As maintainers, this lets you use the test to gain confidence about risky changes while not being held back by a long running (and complex test) for simple changes. Basically, this is an escape hatch to keep the e2e test from blocking progress.
Change the test to utilize replica 3 once either #131 or https://github.com/openshift/release/pull/5047 are finalized.

I'm fine with moving forward like this.

@jarrpa @davidvossel :

Overall looks good. Can you squash the non-vendor commits?

Maybe leave the commit that is taking replica down to 2 separate so that we can revert it later?

Maybe leave the commit that is taking replica down to 2 separate so that we can revert it later?

@obnoxxx @jarrpa I squashed down the commits to what I thought looked like good logical chunks. I think where a few changes that made sense as standalone commits.

/hold cancel

authors: davidvossel jarrpa kshlm obnoxxx

repo: openshift/ocs-operator


text:
initial commit - Creating NooBaa resources in OCS operator

CC: @guymguym
Initial PR integrating NooBaa with OCS operator

Updated vendor and Gopkg to include noobaa-operator
Added default storage class for noobaa OBC provisioner in init controller
Added ensureNoobaaSystem to StorageCluster reconciler

I still need to test it and will appreciate some help with running the operator, mainly where can I get ceph CRDs yamls so I can get it to run

I don't think ensureNooBaaSystem() belongs as part of the StorageCluster Reconcile(). Will it ever be the case that we want more than one NooBaa system in a given installation of OCS? I was thinking we would just have one NooBaa system and then different StorageClasses to make use of different backing stores for different Ceph clusters (if we even go that far).

@jarrpa I think reconcile of StorageCluster is what we want, but lets try to describe the relation between these CR's: 1 StorageCluster => 1 CephCluster + 1 NooBaa - is that how you see it or is there a reason to make anything more complex in these relations?

If anything I think that relationship is too complex already. Currently, 1 StorageCluster -> 1 CephCluster, and we want to be able to expand to multiple CephClusters in the future. But I don't see a need for more than one NooBaa system per OpenShift cluster, as such it makes no sense to tie it to StorageCluster reconciliation.

If anything I think that relationship is too complex already. Currently, 1 StorageCluster -> 1 CephCluster, and we want to be able to expand to multiple CephClusters in the future. But I don't see a need for more than one NooBaa system per OpenShift cluster, as such it makes no sense to tie it to StorageCluster reconciliation.

I'm not sure why we'd want only one Noobaa if we have multiple OCS clusters. Still unsure what are the use cases for multiple OCS clusters, but they may apply to Noobaa as well.

I actually though the opposite - why do you need multiple CephClusters per openshift cluster?
So you want the StorageCluster to just be a wrapper to rook-ceph? OK, so which CRD represents an "OCS deployment" - aka the deployment of ceph and noobaa?

It's not that I want a StorageCluster to JUST be a wrapper for a CephCluster, that's merely what it is right now.
The desire for multiple CephClusters per OCS installation is a request we've had from the beginning that hasn't been completely given up on, mostly for the purposes of completely isolating distinct pools of storage for different tiers of stability and risk management.
I've heard of no such need or request for NooBaa, and I didn't want to introduce it if it wasn't being asked for. As such, I was thinking the OCS operator itself would just create an initial NooBaa system and leave it alone forever. I wasn't seeing anything in the CRD that warranted any monitoring, either.

@jarrpa That's a great idea because it removes another user step! But actually there is no need to do anything at all in the OCS operator. I'll just make the noobaa's operator start by immediately creating the default noobaa CR and then followup by deploying a system. If we ever need more systems we just deploy more operators on other namespaces. Thanks!
+@tamireran @dannyzaken

It's not that I want a StorageCluster to JUST be a wrapper for a CephCluster, that's merely what it is right now.
The desire for multiple CephClusters per OCS installation is a request we've had from the beginning that hasn't been completely given up on, mostly for the purposes of completely isolating distinct pools of storage for different tiers of stability and risk management.
I've heard of no such need or request for NooBaa, and I didn't want to introduce it if it wasn't being asked for. As such, I was thinking the OCS operator itself would just create an initial NooBaa system and leave it alone forever. I wasn't seeing anything in the CRD that warranted any monitoring, either.

Isolating storage classes need to be done via CRUSH map configuration. Stability is not obtained by needing to upgrade and manage a set of clusters - it just adds management overhead on a regular basis (but I do understand the request).

Isolating storage classes need to be done via CRUSH map configuration. Stability is not obtained by needing to upgrade and manage a set of clusters - it just adds management overhead on a regular basis (but I do understand the request).

I'm can't say I disagree with you, merely acknowledging that the issue hasn't been fully put to bed yet, as such I've kept the StorageCluster design flexible.

I'll just make the noobaa's operator start by immediately creating the default noobaa CR and then followup by deploying a system. If we ever need more systems we just deploy more operators on other namespaces. Thanks!

Requiring a unique operator for new instances of the same thing is antithetical to the operator pattern, and sounds like a mess to deal with from an OLM standpoint, but that's a discussion for another time. :) At this stage I don't entirely care what creates the NooBaa system, since we only need one.

I'll just make the noobaa's operator start by immediately creating the default noobaa CR and then followup by deploying a system. If we ever need more systems we just deploy more operators on other namespaces. Thanks!

Requiring a unique operator for new instances of the same thing is antithetical to the operator pattern, and sounds like a mess to deal with from an OLM standpoint, but that's a discussion for another time. :) At this stage I don't entirely care what creates the NooBaa system, since we only need one.

Running a unique deployment of the operator for each NooBaa CR is not a good fit for the operator pattern, particularly as it's implemented in openshift.
If the NooBaa CR is always going to be a singleton (there will only be one per kubernetes cluster), then it would be great for the noobaa-operator to auto-create that CR on startup and take measures to restrict creation of additional instances.
But if it may be desirable to create more than one NooBaa CR on the same kubernetes cluster, then it is best to let something/someone else create those CRs as needed and stick with a single deployment of the operator. In this case, it seems like the ocs-operator is the natural choice to manage those CRs and decide if/when they need to be created.

Running a unique deployment of the operator for each NooBaa CR is not a good fit for the operator pattern, particularly as it's implemented in openshift.

@mhrivnak Why not? Sounds like a very good pattern to me - very simple to use and does exactly what the user intended.

Thanks all.
closing the PR for now. will open a new PR with the storage class and noobaa CSV additions once PR #8 is merged

Running a unique deployment of the operator for each NooBaa CR is not a good fit for the operator pattern, particularly as it's implemented in openshift.

@mhrivnak Why not? Sounds like a very good pattern to me - very simple to use and does exactly what the user intended.

Within the operator ecosystem, we're using the kubernetes API as the way to deploy applications in a declarative way. An operator is three things: a CRD that describes the operand, a controller, and operation logic coded into the controller. In order to deploy Wordpress, I would create an instance of the Wordpress CR and describe in the Spec what I want it to look like. As the persona who is deploying wordpress, I have no involvement with the operator. Much of the point of the pattern is to enable end users to deploy fully-managed workloads without having to worry about who or what is doing the management, giving a similar experience to using a managed service on a public cloud. There is a basic and fundamental expectation that the API to deploy a thing is the CR that represents the thing.
Controllers and operators are inherently designed to reconcile and manage multiple instances of their CR. It's the default behavior. We have created some singletons in a case where we know there should only be one instance of a thing deployed for the entire k8s cluster, but that is already bending the operator pattern in an awkward direction.
Now enter noobaa. The API to deploy noobaa should be the Noobaa CR. That is what users, customers, developers, admins, tooling, the UI, etc will expect. If you instead made the API for deploying noobaa a Deployment, which needs to run a particular operator with a particular image, service account, configuration, etc. that has nothing to do with describing a noobaa deployment. If the user wants to deploy noobaa, let them just describe that through the Noobaa CR.
What if there are no settings on the Spec for them to set? Even in that case, the CR is the API for managing the lifecycle of the noobaa deployment. The Status gives the user an understanding of how noobaa is doing and what management actions may be required. Removing noobaa should simply involve deleting the CR. The full lifecyle of a noobaa deployment should be managed via a CR.
In terms of OCS, we need the ability to deploy rook/ceph, noobaa-operator, and any other operator that joins the product as a single CSV. This is a pattern sometimes called a "meta-operator", where we have one parent operator (ocs-operator in this case) that, from a packaging and installation perspective, is a composite of several other operators. I can elaborate more on this pattern if that would be helpful, but for now let's take it as a given. The installation, upgrade (particularly important), and removal of OCS will be managed through a single CSV. A user will install OCS, and the result will be that all the operators they need will be installed. An upgrade has a definitive state; either the operator upgraded or it didn't. It's either healthy or it's not. Etc. This is what the operator tooling is setup to manage, and so this is a requirement. There is not a way to facilitate installation of additional on-demand operators in that approach, nor do we have another approach we can take that would.
There is also the issue of cost. There is a real cost to the end user for running extra Pods. The total footprint of operators in the openshift ecosystem is quite large, and we should take that seriously. When there is a reasonable opportunity to reduce that footprint, it is a good idea to do so. In this case, it would be a shame to require an extra operator pod per CR unless absolutely necessary.
Lastly, I'm not aware of any other operator that requires a dedicated operator for each CR. Noobaa-operator would be an outlier in that respect, and I strongly encourage you not to follow such a pattern.

Hi @mhrivnak, thanks for the detailed description.
/cc @jarrpa @tamireran @dmesser
I am aware of the current standard of the operator pattern, but I don't see it as perfect yet for every use case. Let me explain:

We are still suggesting that the NooBaa CR will be used to manage the lifecycle, spec and status of the NooBaa deployment - we just auto-create the CR with defaults when the operator starts.

The case of creating wordpress vs creating NooBaa/Ceph is not really similar. Most users will never be interested to deploy NooBaa/Ceph on their own, and will request the deployment from their admin.

The cost of an operator pod with 100m cpu and 100MB memory is negligible for deployments of such storage systems.

The pattern of an operator does not specify that the controller must be a singleton Pod in the cluster. A "controller" is merely a reconcile logic. The suggestion is to deploy that logic as many times as deployments are needed. The vast majority of cases is just 1 deployment and even when there will be more we expect just a couple of deployments per cluster.

Essentially we are using the OLM operator as our meta-operator, and by creating a Subscription we initiate the deployment. OLM install is the day-1 zero-config operation, and any further edits of the NooBaa CR spec are day-2 operations.

The benefit of this pattern is that there is a real separation of the deployments:

from failure tolerance perspective failures of one operator will not affect other systems.
from security perspective the operator can limit its watch and RBAC to just a Role and not require a ClusterRole in order to watch Secrets from all namespaces for example - which IMHO is a bad clusterPermission to require. If your operator reconciles by creating StatefulSets / Pods / Secrets / Services etc, the operator pattern that watches all namespaces will require a ClusterRole with permissions to full control of those core resources on all namespaces, which sounds incredibly risky, isn't it?

I am not convinced that my suggestion is good, but I am also not sure the "standard" operator pattern is good for our deployment use case.
Will be happy to continue the discussion! Thanks

@guymguym I think that the suggestion makes sense. The provided security reasoning is a good point.
I agree that most of the deployments will require only a single  NooBaa deployment, and the suggested solution allows the scale to more than one deployment.
We may change this behavior along the way, based on customer feedback, but currently, our first priority is to deliver the most usable solution, and your suggestion looks like a good fit.
I'm ok if you decide on another solution suggested by others, but suggest to push the first PR asap, to allow longer time for QE.

@tamireran @guymguym regarding your last point - you could argue that running any Operator is attached to some risk. Eventually these are API extensions that introduce global resources (CRD). Your use case of having multiple Operator deployments managing different CRs is supported by OLM.
That said, I concur with Michael that what you describe is somewhat different to the usual UX when deploying and using Operators. Technically there is nothing wrong with auto-creating a CR upon start - this is even what we suggested as some developers looking for patterns to have an initial "Install CRD" that configures some system-level environment. Upon closer investigation we noticed that what they are doing is creating an Operator which deploys another Operator - a story for another time.
Here I would suggest to just have OLM deploy the NooBaa Operator which also deploys a validation admission webhook which eventually makes the decision on whether or not the CR would be allowed. This is how you can implement singleton behavior. Also this retains the usual UX (Operators don't change the state until their APIs are used) and still allows for the behavior you are looking for.

reopening the PR
after a discussion with @tamireran there should be a 1:1 relation between OCS instance and noobaa

@jarrpa I checked again with @tamireran on the product design, and as I understand what we need to provide is that the OCS operator will start, and then there will be an OCS CR that will deploy both Ceph and NooBaa. If you have questions on the design we should verify it with Eran. Thanks

This PR still does not ensure a 1-to-1 mapping between OCS installation and NooBaa system. The NooBaa system needs to be deployed during the OCSInitialization reconcile, not the StorageCluster reconcile.

@jarrpa, The desired behavior is to deploy the operator in parallel or after the storagecluster reconcile. There is a dependency on the storage class as well as RGW on-prem, and I want to avoid a situation where we have a pending NooBaa system until we have the storage class deployment.
Please approve the PR
Thanks

This PR still does not ensure a 1-to-1 mapping between OCS installation and NooBaa system. The NooBaa system needs to be deployed during the OCSInitialization reconcile, not the StorageCluster reconcile.

@jarrpa, The desired behavior is to deploy the operator in parallel or after the storagecluster reconcile. There is a dependency on the storage class as well as RGW on-prem, and I want to avoid a situation where we have a pending NooBaa system until we have the storage class deployment.

All operators that are part of OCS will be deployed at approximately the same time by OLM. We should not depend on the order of operators getting deployed. But this PR doesn't seem to involve deploying operators, so maybe you are talking about the operands?
If noobaa-operator cannot do what it needs until some other things exist, can it simply wait for those things to exist? It's normal for an operator to either:

wait in main.go until a particular API becomes available, then start controllers
re-queue a resource that's being reconciled if the operator needs to wait more time for something to happen

Is one of those not sufficient for the situation you mentioned?

There is no dependency here that would prevent the NooBaa operator from starting, so they will still deploy in parallel. Moving this function to the OCS initialization will introduce no further dependency, and in fact will allow NooBaa to start without waiting for Ceph configuration, which I believe is desired. Further, the StorageClasses can be arbitrarily created at any time, the only consequence is that any PVC requests will remain pending until the configured provisioner exists / succeeds.
Thus, I still want this to be part of the operator (basically, product) initialization, rather than a StorageCluster initialization.

@jarrpa but ocs initialization means that this is part of the operator install flow, right? This is not what product design requires. It requires us to i stall NooBaa with Ceph when instantiating an OCS deployment. Can you please explain how your preference meets this requirement? Thanks?

Okay, so I've acquiesced to the current proposed design of having one NooBaaSystem per StorageCluster. However, I still believe this PR is incorrect. The reason is because we are also implementing a StorageClassInitialization CRD that will take care of doing things like creating the default CephBlockPool and RBD StorageClass (among other things). As such, That's the CR that would have the information on what StorageClass will exist, and not the StorageCluster CR. So, we need to be creating the NooBaaSystem there.... unfortunately, that "there" doesn't exist yet. Thus, I'm putting a hold on this PR until we get that initialization CRD created. I'll try and get that moving this week.
/hold

Okay, so I've acquiesced to the current proposed design of having one NooBaaSystem per StorageCluster. However, I still believe this PR is incorrect. The reason is because we are also implementing a StorageClassInitialization CRD that will take care of doing things like creating the default CephBlockPool and RBD StorageClass (among other things). As such, That's the CR that would have the information on what StorageClass will exist, and not the StorageCluster CR. So, we need to be creating the NooBaaSystem there.... unfortunately, that "there" doesn't exist yet. Thus, I'm putting a hold on this PR until we get that initialization CRD created. I'll try and get that moving this week.

Since we're getting closer to a clear picture, it would help to write down what StorageClasses are going to get created and when. That would certainly help answer some of my questions about how things relate to each other and how hard we need to try to do things in a specific order.
With as much detail as you provide:

What StorageClasses get created when the operator initializes?
What StorageClasses get created when reconciling a StorageCluster cr? What information from the StorageCluster Spec will be used to create those StorageClasses?

Since we're getting closer to a clear picture, it would help to write down what StorageClasses are going to get created and when. That would certainly help answer some of my questions about how things relate to each other and how hard we need to try to do things in a specific order.

Let's move this conversation out of this PR.
I've created #29 to implement the StorageClusterInitialization CRD, which is what should be doing the NooBaa initializations as well. I'll update this PR again when that merges.

#29 has merged, work on this can now resume.
/hold cancel

@dannyzaken this needs rebase / update after some changes in master.

@dannyzaken @guymguym ping

@obnoxxx there are too many conflicts to update this PR. I will make a new PR and reference this PR from it.
I am still testing, should have the new PR later today

@dannyzaken :

@obnoxxx there are too many conflicts to update this PR. I will make a new PR and reference this PR from it.

You can also just locally rebase/rework and force-push over this one. But if you want to create a new PR, that would work too.

I am still testing, should have the new PR later today

OK, thanks!

@obnoxxx thanks. I already implemented on a different branch so I'll just create a new PR

created PR #77 to replace this PR. closing

authors: dannyzaken dmesser guymguym jarrpa mhrivnak mykaul obnoxxx tamireran


text:
odo debug: provide inner loop scripts with DEBUG support for nodejs and java

parent  #822
add innerloop scripts for java and modify nodejs scripts to support $DEBUG_MODE $DEBUG_PORT
as described in odo debug proposal -  https://github.com/openshift/odo/pull/1995
This requires https://github.com/openshift/odo-supervisord-image/pull/27 to be merged first, as it provides necessary groundwork to for using custom inner loop scripts. See https://github.com/openshift/odo-supervisord-image/pull/27 for more details

Done in https://github.com/openshift/odo-init-image/pull/27 and https://github.com/openshift/odo/pull/1974
/close

repo: openshift/odo


text:
Example in odo service create -h doesn't work

/kind bug

What versions of software are you using?
Operating System:
Fedora
Output of odo version:
odo v2.0.1 (a9fff7da4)
Server: https://api.cluster-rdu-d666.rdu-d666.example.opentlc.com:6443
Kubernetes: v1.17.1+3f6f40d
How did you run odo exactly?
odo service create -h
That shows this:
Examples:
  # Create new postgresql service from service catalog using dev plan and name my-postgresql-db.
  odo service create dh-postgresql-apb my-postgresql-db --plan dev -p postgresql_user=luke -p postgresql_password=secret

But when I run it, I get:
 ✗  invalid service name, use the format <operator-type>/<crd-name>

More to the point, I'm unable to create any service catalog backed service, all showing me the message about the format requiring operator-type/crd-name, which isn't relevant in the service catalog case.
Actual behavior
Expected behavior
Any logs, error output, etc?

This would be handled as a part of #4242.

@dharmit is it okay to add the lifecycle/active label here? I see you have been working on the related issue #4242, so this issue is definitely not stale.

@jdob I understand that this issue is for cli help message. So my response is tangential. Can you see if a recently added piece of documentation helps you with java+database example - https://odo.dev/docs/deploying-java-app-with-database/?
@valaparthvi it's
/lifecycle frozen

authors: dharmit jdob valaparthvi


text:
Use explicit rhel7 version

Using rhel7 only doesn't have to correspond with what we expect (latest rhel7), so we need to be more explicit here.

[test]

authors: bparees hhorak

repo: openshift/postgresql


text:
OCM-7184 | feat: Moving create machinepool and nodepool functions from cmd -> pkg

Moving create nodepool + machinepool  functions from cmd directory to the pkg/machinepool directory, updating tests to increase coverage to +70%
OCM-7184

/retest

/retest

/hold

Holding for cut of 1.2.41 release.

/retest

/retest

/retest

/retest

/hold cancel

/lgtm

authors: ciaranRoche den-rgb robpblake

repo: openshift/rosa


text:
Add basic etcd insufficient members runbook

This adds a basic starting runbook for etcdInsufficientMembers which is a critical alert. This will be improved later on, either by etcd team or SRE folks or by feedback from our end users.
@RiRa12621 @hexfusion please take a look, as mentioned we have a lot of runbooks to add so these are just basic things and we will improve in follow ups.

/lgtm
Thanks!

repo: openshift/runbooks


text:
fix syntax error: unexpected range for go1.3.3

when go version is 1.3.3, compile with error:
[root@openshift-144 source-to-image]# make
hack/build-go.sh
++ Building go targets for linux/amd64: cmd/sti
github.com/openshift/source-to-image/pkg/docker
_output/local/go/src/github.com/openshift/source-to-image/pkg/docker/docker.go:407: syntax error: unexpected range, expecting {
_output/local/go/src/github.com/openshift/source-to-image/pkg/docker/docker.go:410: syntax error: argument to go/defer must be function call
_output/local/go/src/github.com/openshift/source-to-image/pkg/docker/docker.go:412: non-declaration statement outside function body
_output/local/go/src/github.com/openshift/source-to-image/pkg/docker/docker.go:413: syntax error: unexpected }

lgtm but will let @gabemontero give the sign off since he wrote the original code and might know something i don't.

Ah yes - I originally had something like the change proposed here, but then the travis and jenkins jobs failed because hack/verify-gofmt.sh.
In fact, as you can see from the failed travis job, that failure again occurred:
$  ./hack/verify-gofmt.sh
!!! gofmt needs to be run on the following files:
./pkg/docker/docker.go
Try running 'gofmt -s -d [path]'
The command "./hack/verify-gofmt.sh" failed and exited with 1 during .
So the change from @mdshuai is not going to fly.
I see there is the --tags option for go build, and that it provides #ifdef type capabilities, but @bparees - do we want to go down that path here?

Weird that it's a gofmt error, but no we definitely do not want to add
ifdef support for something like this. We don't make any claims about
supporting go 1.3 anyway.
Thanks for the background.
Ben Parees | OpenShift
On Aug 19, 2015 9:34 AM, "Gabe Montero" notifications@github.com wrote:

Ah yes - I originally had something like the change proposed here, but
then the travis and jenkins jobs failed because hack/verify-gofmt.sh.
In fact, as you can see from the failed travis job, that failure again
occurred:
$ ./hack/verify-gofmt.sh
!!! gofmt needs to be run on the following files:
./pkg/docker/docker.go
Try running 'gofmt -s -d [path]'
The command "./hack/verify-gofmt.sh" failed and exited with 1 during .
So the change from @mdshuai https://github.com/mdshuai is not going to
fly.
I see there is the --tags option for go build, and that it provides #ifdef
type capabilities, but @bparees https://github.com/bparees - do we want
to go down that path here?
—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/267#issuecomment-132599823
.

@mdshuai is there a particular need to build this under go 1.3.3 or can we close this?
if you absolutely must fix the 1.3.3 compilation i think you need to change the code to:
		for _, dummy := range signalChan {

And run hack/verify-gofmt.sh after the change.
On Wed, Aug 19, 2015 at 10:28 AM, Ben Parees notifications@github.com
wrote:

@mdshuai https://github.com/mdshuai is there a particular need to build
this under go 1.3.3 or can we close this?
if you absolutely must fix the 1.3.3 compilation i think you need to
change the code to:
for _, dummy := range signalChan {
—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/267#issuecomment-132618670
.

@bparees @gabemontero On our ose env(rhel7), the default go version is go1.3.3. Sometime we need compile the s2i binary on those env. There is really some different in go1.4 and go1.3 about "for ... range" usage.
https://golang.org/doc/go1.4#forrange
For the error @gabemontero meet when run hack/verify-gofmt.sh is cause by this.
https://golang.org/cmd/gofmt/#hdr-The_simplify_command
I have update the code for range a channel.After change have run hack/verify-gofmt.sh and no issue.
If you think we don't  want to support for go1.3.3, then you can close this. help review , thanks.

Igtm
@bparees while I think it should be fine, I'll double check the new --run=true option once this is upstream, make sure it still does what we want with this change

@gabemontero you can cherry pick this change and build it locally to test it before we merge.
it also makes me think we should have a test case in s2i that runs on merge for this function, if we're worried about it.

@bparees  OK, I'll try to cherry pick and see (this will be my first official cherry-pick attempt)
On the test, since the --run=true requires a Ctrl-C to exit the run, and the "verification" is examining the images logs, it gets a little tricky.  But probably I'm missing something that you can educate me on.

can't you kill -9 the s2i process and that should kill everything?
On Thu, Aug 20, 2015 at 11:34 AM, Gabe Montero notifications@github.com
wrote:

@bparees https://github.com/bparees OK, I'll try to cherry pick and see
(this will be my first official cherry-pick attempt)
On the test, since the --run=true requires a Ctrl-C to exit the run, and
the "verification" is examining the images logs, it gets a little tricky.
But probably I'm missing something that you can educate me on.
—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/267#issuecomment-133053189
.

--
Ben Parees | OpenShift

Sure, from the shell script, run sti in the background, then kill it at
some point, but a) how to determine when to kill it (i.e. let it run long
enough) ... perhaps via docker ps and/or docker exec ... ??
On Thu, Aug 20, 2015 at 11:39 AM, Ben Parees notifications@github.com
wrote:

can't you kill -9 the s2i process and that should kill everything?
On Thu, Aug 20, 2015 at 11:34 AM, Gabe Montero notifications@github.com
wrote:

@bparees https://github.com/bparees OK, I'll try to cherry pick and
see
(this will be my first official cherry-pick attempt)
On the test, since the --run=true requires a Ctrl-C to exit the run, and
the "verification" is examining the images logs, it gets a little tricky.
But probably I'm missing something that you can educate me on.
—
Reply to this email directly or view it on GitHub
<
https://github.com/openshift/source-to-image/pull/267#issuecomment-133053189
.

--
Ben Parees | OpenShift
—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/267#issuecomment-133054867
.

Redirect the output to a file and monitor the output?
Ben Parees | OpenShift
On Aug 20, 2015 11:45 AM, "Gabe Montero" notifications@github.com wrote:

Sure, from the shell script, run sti in the background, then kill it at
some point, but a) how to determine when to kill it (i.e. let it run long
enough) ... perhaps via docker ps and/or docker exec ... ??
On Thu, Aug 20, 2015 at 11:39 AM, Ben Parees notifications@github.com
wrote:

can't you kill -9 the s2i process and that should kill everything?
On Thu, Aug 20, 2015 at 11:34 AM, Gabe Montero <notifications@github.com
wrote:

@bparees https://github.com/bparees OK, I'll try to cherry pick and
see
(this will be my first official cherry-pick attempt)
On the test, since the --run=true requires a Ctrl-C to exit the run,
and
the "verification" is examining the images logs, it gets a little
tricky.
But probably I'm missing something that you can educate me on.
—
Reply to this email directly or view it on GitHub
<

https://github.com/openshift/source-to-image/pull/267#issuecomment-133053189

.

--
Ben Parees | OpenShift
—
Reply to this email directly or view it on GitHub
<
https://github.com/openshift/source-to-image/pull/267#issuecomment-133054867
.

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/267#issuecomment-133056411
.

Yeah and maybe grep for an expected string or something .... I'll open a separate issue to look into it - thx

On Aug 20, 2015, at 11:58 AM, Ben Parees notifications@github.com wrote:
Redirect the output to a file and monitor the output?
Ben Parees | OpenShift
On Aug 20, 2015 11:45 AM, "Gabe Montero" notifications@github.com wrote:

Sure, from the shell script, run sti in the background, then kill it at
some point, but a) how to determine when to kill it (i.e. let it run long
enough) ... perhaps via docker ps and/or docker exec ... ??
On Thu, Aug 20, 2015 at 11:39 AM, Ben Parees notifications@github.com
wrote:

can't you kill -9 the s2i process and that should kill everything?
On Thu, Aug 20, 2015 at 11:34 AM, Gabe Montero <notifications@github.com
wrote:

@bparees https://github.com/bparees OK, I'll try to cherry pick and
see
(this will be my first official cherry-pick attempt)
On the test, since the --run=true requires a Ctrl-C to exit the run,
and
the "verification" is examining the images logs, it gets a little
tricky.
But probably I'm missing something that you can educate me on.
—
Reply to this email directly or view it on GitHub
<

https://github.com/openshift/source-to-image/pull/267#issuecomment-133053189

.

--
Ben Parees | OpenShift
—
Reply to this email directly or view it on GitHub
<
https://github.com/openshift/source-to-image/pull/267#issuecomment-133054867
.

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/source-to-image/pull/267#issuecomment-133056411
.

—
Reply to this email directly or view it on GitHub.

ok ... cherry-picked the change, verified still good with --run=true.  @bparees let's merge

[merge]

thanks @gabemontero and @mdshuai

authors: bparees gabemontero mdshuai

repo: openshift/source-to-image


text:
Update version to v1.1.2

Updated for v1.1.2 operator release.

/lgtm

/approve

/retest

/retest

/lgtm cancel
@khrm @piyush-garg @savitaashture @nikhil-thomas what is the way forward here ?

authors: khrm nikhil-thomas piyush-garg vdemeester

repo: openshift/tektoncd-pipeline-operator


text:
Adding rupalibehera user to admin list

I am working on moving jobs to CICO I will need this

Can one of the admins verify this patch?

[test]

authors: centos-ci riuvshin rupalibehera

repo: openshiftio/openshiftio-cico-jobs


text:
Muscle should not be open cylinders

Steps to reproduce
Open a model and look at the end of a muscle.
Expected result
Muscle is a cylinder with closed ends.
Actual result
Muscle is a straw. This issue was already raised in #139, which was closed (the Issue was closed, not the muscle cylinder). If not deemed important, perhaps this Issue can remain open, filed under the "Future Release" milestone?

Environment and GUI version
AppVeyor artifact OpenSim-e7831221-2018-03-06.zip

@aymanhab Can you comment on this one? I think the other issue was closed because the issues in the original checklist were completed.

Flat caps should be easy to put in and at very little cost to rendering time, Circular caps a re a bit more involved and likely should wait till after 4.0 unless I have volunteers. After flatcap is inplace I can describe what it takes.

Thanks ... I will edit the title of this a little and put in the 4.0 milestone.

@aymanhab we tried flat caps for ISB last year and they did not work out as expected. We ended up reverting to open cylinders since that was the best we could do in the short-term then. Since you need path points to be editable, I would just make them spheres of the same color and radius as the muscle.

I managed to cap the ends of the muscles with spheres. It's a pretty big hack at the moment (which should say a lot coming from me :wink:) but I think it could be implemented cleanly without too much additional work. Happy to discuss.

Looking great, i was planning to address this as part of the pathpoint
editing issue on my list, would love to see how yiur solution works, any
overhead incurred during viewing, animation, color changes etc. Let's
discuss in person today.
On Sun, May 6, 2018, 11:39 PM Tom Uchida notifications@github.com wrote:

I managed to cap the ends of the muscles with spheres. It's a pretty big
hack at the moment (which should say a lot coming from me 😉) but I think
it could be implemented cleanly without too much additional work. Happy to
discuss.
https://user-images.githubusercontent.com/4203505/39688182-66f398d2-5186-11e8-937f-7a89b0eaf914.jpg
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/opensim-org/opensim-gui/issues/581#issuecomment-386971911,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADyccM4uKTm0lf0Rxb9mToYG92Y7byEEks5tv-wdgaJpZM4SlNwY
.

Thanks. I didn't know you were working on it. I can't assess impact on performance because the AppVeyor artifact I hacked didn't play smoothly to begin with. I think the most elegant solution would be to draw hemispheres with the same number of slices as the muscle skin and merge the meshes---but it's much simpler to just draw spheres at the endpoints. Not sure whether you want to implement via JSON instructions or entirely on the JavaScript side as I have done.

Fixed on branch https://github.com/opensim-org/opensim-gui/tree/visual_path_edit

Looks nice! There's a bit of Z-fighting, but that can be addressed (perhaps in 4.1) by rendering the model plus local part of the floor with tighter clipping planes.

As of OpenSim 1e476f2-2018-06-13 muscles now draw as open tubes again. It is still present in OpenSim 461c0df-2018-06-22. Look terrible IMHO and I don't recall the team decision agreeing to bring it back.

authors: aseth1 aymanhab jenhicks tkuchida

repo: opensim-org/opensim-gui